groue / GRDBCombine

GRDB ❤️ Combine
MIT License
223 stars 16 forks source link

How to prevent an infinite loop when the current view is updating the database? #40

Closed simensol closed 4 years ago

simensol commented 4 years ago

I have a View with a segmented control picker which is used to update some column named status in my database.

Based on the demo app, I have created the following publisher:

// Publisher
func myPublisher(taskId: UUID) -> DatabasePublishers.Value<Task?> {
    ValueObservation
        .tracking { db in
            try Task
                .select(Task.Columns.status)
                .filterId(taskId)
                .fetchOne(db)
        }
        .removeDuplicates()
        .publisher(in: database)
}

The publisher is used in my ViewModel:

@Published var taskStatus: TaskStatus {
    didSet {
        // Updates database...
    }
}

// Subscriber
init(task: Task) {
    Current.tasks()
        .myPublisher(taskId: task.id)
        .fetchOnSubscription()
        .catch { _ in Empty() }
        .sink { [weak self] in
            if let task = $0 {
                // Update status
                self?.taskStatus = task.status
            }
        }
        .store(in: &cancellableSet)
}

On initialization, the picker gets its initial value from @Published var taskStatus as expected. However, when the picker's value is changed, the following happens:

  1. The @Published var taskStatus's didSet{} observes the change, and updates the status column in the database.
  2. myPublisher() observes the change in the status column, and the subscriber is triggered.
  3. When the subscriber is triggered, @Published var taskStatus is updated, and its didSet{} is fired, updating the database (again).
  4. This starts an infinite loop.

Is it possible to make the publisher (or subscriber) aware of the source of the database update, so that I can break this infinite loop?

groue commented 4 years ago

Hello @simensol,

I'm not quite versed in data-flow design in Combine apps, so I won't comment on your setup.

I don't understand how you can get an infinite loop with the removeDuplicates() modifier.

Can you investigate? Maybe share a stacktrace from inside this infinite loop, hoping it's not totally obfuscated with Combine guts?

simensol commented 4 years ago

Thanks anyway, @groue! I will investigate and follow up.

simensol commented 4 years ago

I didn't managed to fix the cause. By comparing the old Task record to the updated one, the second update in the loop were never executed, hence, breaking the loop:

// ViewModel.swift
@Published var taskStatus: TaskStatus {
    didSet {
        Task.updateStatus()
    }
}

// Tasks.swift
func updateStatus(_ existingTask: Task, newStatus: TaskStatus) throws -> Bool {
    _ = try database.write { db in
        if var task = try Task.fetchOne(db, key: existingTask.id) {
            let isTaskUpdated = try task.updateChanges(db) {
                $0.status = newStatus
            }

            #if DEBUG
            if isTaskUpdated {
                print("Task.status was updated")
            } else {
                print("Task.status was not updated")
            }
            #endif
        }
    }
}

A more robust solution probably exists. However, I havn't found it (yet).

groue commented 4 years ago

OK, @simensol. I'd be happy if you could share a reproducible case, so that someone else can keep on investigating (probably me). The sample code you provide is not enough to reproduce the issue and perform adequate debugging.

simensol commented 4 years ago

I will see what I can find, @groue. I'm the only developer on my project, so the git commits are unfortunately a bit messy.

simensol commented 4 years ago

I'm still working on this issue. I think the problem is related to SwiftUI, and not GRDBCombine, but I'm not sure yet. When I initiate the app, 4 TransactionObservation are created. However, after opening the same View 20 times, the number of TransactionObservation has increased to 24. I call Cancel() on the cancellableSet. Is the increased number of TransactionObservation expected, or should they be removed when I call Cancel() on the cancellableSet? And does the increased number of TransactionObservation represent an issue?

Screenshot 2020-05-01 at 11 08 59

EDIT:

It seems the issue was related to nested views in SwiftUI. When a view is closed, the subscriber is not automatically canceled. So I have to call Cancel() on the cancellableSet manually when a view is closed.

groue commented 4 years ago

Closing due to lack of activity. See you later in the GRDB repository, because GRDBCombine is no longer an independent library!