groue / GRDB.swift

A toolkit for SQLite databases, with a focus on application development
MIT License
6.62k stars 680 forks source link

Best practices for updates in a DiffableDataSource #1086

Closed tcwalther closed 2 years ago

tcwalther commented 2 years ago

I'm wondering how to best use GRDB structs with DiffableDataSource. I've already learned that, since structs are value types, it is better to build datasources that store the ID, and not the value. This is also recommended directly by Apple in their WWDC 2021's video "Make blazing fast lists and collection views": https://developer.apple.com/videos/play/wwdc2021/10252/

This technique makes sure that ValueObservations for lists can be correctly diff'ed by DiffableDataSource, and works well. However, because DiffableDataSource now only compares the id, it doesn't find out which cells will need a refresh.

Let's take a list of Authors as an example.

struct Author: Identifiable, TableRecord {
    var id: UUID
    var name: String
    var isFavorite: Bool
}

We can create the data source now via:

var dataSource: UICollectionViewDiffableDataSource<String, Author.ID> = ...

The particular use case I have at hand is that an Author can become a favorite. For that, there is a swipe action on the cell. Swiping calls AppDatabase.shared.saveAuthor(author). Now I need to tell the cell to redraw. My ValueObservation on var allAuthors - the underlying variable for my datasource - will now trigger. Without the solution of having a dataSource on Author.ID, that update would look like a remove+insert operation. With Author.ID, DiffableDataSource doesn't replace the existing cell, but also doesn't update it - the ID is the same, after all.

Looking at Apple's example code from the link above, it seems that they call snapshot.reconfigureItems([...]), an iOS 15+ only API, to reload a cell. That doesn't feel particularly elegant, and it's iOS 15+ only.

In the past, I listened to an updatedAuthor notification in each cell, and then each cell could decide whether it was meant by that update. I can of course still implement that pattern, but I was hoping that ValueObservations would free me from it, at least to an extent. The other option would be to start a ValueObservation for each cell, but that seems like a terrible idea performance wise - imagine scrolling through a list of 1000 records; you'd be creating and discarding ValueObservations all the time.

Do you have any advice on best practices on how GRDB should interact with DiffableDataSources?

Thanks for your reply. Also, I believe this is the last question I have for now while I am digging into GRDB. Thank you so much for taking the time to reply to my two previous questions, too, I truly appreciate it.

groue commented 2 years ago

Hello @tcwalther,

Maybe you can update all visible cells on each update of the data source from the value observation?


Diffable data sources can not take care of item updates, because their only requirement is ItemIdentifierType: Hashable. If only they had used ItemIdentifierType: Identifiable & Hashable, they could have distinguished changes in the list of ids from changes for each individual id.

So we're all left with questions when we do not want a modified item to be handled as a remove+insert, and follow the recommended practices.

I was hoping that ValueObservations would free me from it.

I was hoping diffable data sources would free us from it. After all, this is possible: RxDataSources correctly distinguish identity from equality, and do the expected job. Maintaining a diff algorithm is hard, so I preferred pushing it out of the library.

This has not always been the case. In previous GRDB versions (`0.54.0..<5.0.0`), we had a class named [FetchedRecordsController](https://github.com/groue/GRDB.swift/blob/v4.14.0/Documentation/FetchedRecordsController.md) which would mimic [NSFetchedResultsController](https://developer.apple.com/documentation/coredata/nsfetchedresultscontroller), and its ability to distinguish updates from remove+insert. Unfortunately, this tool was limited: - inefficient diffing algorithm - unable to deal with sections - only able to deal observe a single request - only able to deal observe arrays of records As time passed, the Apple platforms were clearly evolving towards the split of _producing lists_ from _diffing lists_. The divorce is total in SwiftUI. Eventually I decided to focus on producing lists (actually observing any kind of value with `ValueObservation`), and stop dealing with diffs (now an external concern). FetchedRecordsController was eventually removed in GRDB 5.

The other option would be to start a ValueObservation for each cell

Don't do that, this would be terribly inefficient, as you clearly see.

The second reason why this is a bad idea is that you should generally avoid merging the values of several ValueObservations on the screen. Since each observation runs independently from each other, they access different states of the database. As two observations A and B fetch their fresh values, nothing prevents a concurrent write to sneak in between. A and B would publish, at different times, values that can break you database invariants. As concurrent writes eventually stop, A & B will eventually catch up. But for a while, you could have to deal with a list of ids (1, 2, 3), and no author for id 1, for example. Here be dragons.

A good rule of thumb is to feed one "screen" with one single ValueObservation that performs as many requests as needed. And if you really want to use several ValueObservations, fine, but make sure that you do not rely on invariants.

If this surprises you, try to keep in mind that this library is just a convenience layer on top of SQLite. The fundamental behaviors of SQLite are there, you can touch them. In particular, one needs a transaction in order to use the database in an isolated fashion. Those transactions translate, one-to-one, with GRDB pairs of brackets:

try dbQueue.read { db in /* isolated */ }
try dbQueue.write { db in /* isolated */ }
try ValueObservation.tracking { db in /* isolated */ }

When you're isolated, all your database invariants hold. But two distinct transactions can disagree on the content of the database.

tcwalther commented 2 years ago

@groue thank you so much for your detailed and very thorough explanation.

Your view on Apple's DiffableDataSource is very interesting, and for me personally at least enlightening. I've only been working on iOS for a bit more than a year after several years of absence on this platform, and I often find it difficult to understand how the different pieces fit (or don't fit) together. For me, personally, it's very interesting to see Apple advocating value types in their 2021 video, when in every Apple example before that which I could find, their DiffableDateSource took reference types instead.

I'm quite hesitant when it comes to pulling in further dependencies such as RxDataSources, although I find your link to it very helpful for general understanding.

Also, thanks for your strong advice on using just one view of the database. This totally makes sense, and I'm grateful for your reminder.

Updating the cell via notifications works well. I just need to figure out how to fix the delete animation in my CollectionView - which I believe is one last bit of DiffableDataSource to understand. But I'm confident I'll walk away with a much better understanding of how the pieces work, and how they work together.

Thank you so much for your advice. Feel free to close this issue.

tcwalther commented 2 years ago

I figured out the delete animation problem - hopefully this will be useful for other people in the future.

Right now, I'm populating my collection view as the result of observing a table. Say, ValueObservation { try Author.fetchAll(db) }. In one collection view, I show all authors, in another, I only show favourite authors (see example in the opening post above).

The swipe action for (de-)marking an author as favourite is:

listConfig.leadingSwipeActionsConfigurationProvider = { [weak self] indexPath in
    guard
        let authorId = self?.dataSource.itemIdentifier(for: indexPath),
        let author = self?.authorsById[authorId]
    else { return nil }

    let title = author.isFavorite ? "Unfavorite" : "Favorite"
    let action = UIContextualAction(style: .normal, title: title) { _, _, completion in
        var mutableAuthor = author
        mutableAuthor.isFavorite.toggle()
        if AppDatabase.shared.save(mutableAuthor) {
            // This completion leads to weird behavior; see discussion below
            completion(true)
        } else {
            completion(false)
        }
    }
}

Calling AppDatabase.shared.save(mutableAuthor) triggers a refresh in the value observation, which will eventually (i.e., almost instantly) call dataSource.applySnapshot(...). In the "favourite authors" list, changing this attribute results in removing the author from the list.

However, in the code above, completion(true) is called before the new snapshot is applied. As a result, the cell is moved back to its original position, right before applySnapshot removes it from the list. This results in weird flickery behaviour. If completion(true) is not called before applySnapshot removes the item from the list, the animation looks as expected.

I'll now find a way to make sure the completion closure is not called when the item is about to be deleted from the list.

groue commented 2 years ago

Yes, RxDataSources is quite to the point, but also... very alien, and not at all straightforward to plug in or remove (please do not be offended, dear authors of RxDataSources).

I interpret the shift of Apple recommendations from classes to structs to major improvements in the compiler and the core libraries. Structs are now much cheaper.

And thanks for welcoming my long answers ;-)

groue commented 2 years ago

I'll now find a way to make sure the completion closure is not called when the item is about to be deleted from the list.

This is totally uncharted territory to me, because I haven't used those UIKit apis yet :-)

I'm not sure this will be very easy! Please report your eventual wishes if you can express them.

tcwalther commented 2 years ago

I'll now find a way to make sure the completion closure is not called when the item is about to be deleted from the list.

This is totally uncharted territory to me :-)

I'm not sure this will be very easy!

I can predict in advance - before the action happens - whether an action will lead to cell removal. It doesn't make the code super elegant, but it is possible.

Interestingly, in my Core Data code, saving the author triggers an instant, blocking refresh of the authors list, before giving control back to the UI. As such, applySnapshot() is executed before completion(true), and the problem doesn't exist. I need to dig a bit more whether that was just the result of me writing code in a certain way, or whether that was inherent to Core Data's design. (update: that was just accidental, not a fundamental design feature of CoreData)

An equivalent design could be introduced by GRDB where a save function only returns after all ValueObservations have executed their callback. I don't know whether that's a good idea, though.