[Core Data] adapter.performUpdates does not trigger any diffing on NSManagedObjects

[pappalar]

Hello,

I have a question about the framework, because from the doc I would expect a behaviour, but it does not happen.
I cannot provide a example project, because my app is huge and uses CoreData etc.. and it would take me a lot of time to set it up.

The idea is the following:

• I display CoreData object with IGListKit based on a timestamp
• CoreData objects are updated with background network requests
• The AdapterDataSource is using a NSFetchedResultController to monitor for object changes
• When the NSFetchedResultControllerDelegate is informed of CoreData changes it calls: adapter.performUpdates

What happens is:

• Datasource method objects(for listAdapter) is called again, the objects obtain the new correct order
• UI is animated and sections shuffle around in the right order

however:

• IGListDiffable Objects isEqual() method is never called
• Section Controller are not reloaded

I would expect that calling performUpdates:

• 🆗 the datasource refreshes list of object
• 🆗 the UI is animated
• ⚠️ the objects are checked to see if they have changed
• ⚠️ the content of the section controller is recalculated if needed
• ⚠️ UI content is updated correcly

I could call reloadData but I would lose the animation, and the purpose of using the framework.

There is something I understood wrong, or that I am not doing?

You can reproduce this in @rnystrom sample for RayWendlich:
https://koenig-media.raywenderlich.com/uploads/2016/12/MarsLink_Final.zip

I put the code here:
https://github.com/racer1988/IGListKitPerformUpdates

---

EDIT:

Using a IGListReloadDataUpdater instead of a IGListAdapterUpdater provides the expected result, without animation and losing the scroll position (due to reload data)

I hence believe there is a bug (?) in the IGListAdapterUpdater that do not take in account model changes but just inserts and delete. Is this the expected behaviour?

Do I need to write my own adapter to manage the changes to the datamodel?

[pappalar]

In my opinion this code is now working as I would expect:

#pragma mark - IGListUpdatingDelegate

static BOOL IGListIsEqual(const void *a, const void *b, NSUInteger (*size)(const void *item)) {
    const id<IGListDiffable> left = (__bridge id<IGListDiffable>)a;
    const id<IGListDiffable> right = (__bridge id<IGListDiffable>)b;
    return [[left diffIdentifier] isEqual:[right diffIdentifier]];
}

If I take a instance of a object and I edit it, both a and b will contain the change, so the object will always been seen as equal, even if it changed between 2 interactions of the datasource in the collection view.

[rnystrom]

@racer1988 You're right about the equality check being correct. Core Data uses mutable models when things change in the background, unfortunately. That makes it a little difficult when using IGListKit. B/c the pointers are the same, its impossible to see that the object has "changed" b/c there's no previous state to go off of.

Since NSManagedObject doesn't conform to NSCopying, this isn't super easy.

My immediate recommendation is to use a different type of immutable model for your lists. You can construct these in the IGListAdapterDataSource method by iterating your Core Data models and creating immutable versions of them.

Two ways to update your section controllers when an NSManagedObject changes:

• At the VC level, when the NSFRC says objects are reloaded, call -[IGListAdapter reloadObjects:] on all the objects that reloaded.
• Have your section controllers listen for changes (through KVO, notifications, or something) and call [self.collectionContext reload...:self] inside it

Core Data mutability is super tricky (and personally I think its a bad design) so going w/ an immutable model strategy might be better in the long run.

[rnystrom]

@racer1988 worth putting out there, if you come up w/ a solid architecture/design for this, it'd be an amazing repo/tutorial/blog post or something. This is definitely a common issue and will only become more common.

[jessesquires]

@rnystrom -- CoreData is beautiful, but misunderstood. 😉

There should only ever be one instance of any managed object in CoreData, so you'll need to use some kind of proxy objects to make this work with ListKit.

@racer1988 -- Maybe something like this will work.

You could generate token objects to hand over to ListKit:

final class ManagedObjectToken {
    let objectId: NSManagedObjectID
    let hasChanges: Bool

    init(objectId: NSManagedObjectID, hasChanges: Bool) {
        self.objectId = objectId
        self.hasChanges = hasChanges
    }
}

// also need to implement Equatable for ManagedObjectToken

extension NSManagedObject {
    func token() -> ManagedObjectToken {
        return ManagedObjectToken(objectId: self.objectID, hasChanges: self.hasChanges)
    }
}

Your ListKit objects will be ManagedObjectTokens so this is what your SectionController will receive, but you'll probably need the full NSManagedObject to configure cells, etc. inside the SectionController.

I can think of 2 options to deal with this:

1. Your SectionController can have a delegate/dataSource to ask for its full managed object (using the token's NSManagedObjectID
2. Or, you can pass your SectionController the managedObjectContext and fetch the object directly from there (again via the token's NSManagedObjectID)

I'm not 100% sure if what I have above will work, but something similar should.

[pappalar]

@rnystrom @jessesquires thank you for your answers, now the whole thing and what is supposed to happen makes perfect sense.

I added a bit of code examples in #461.

I will implement this in my work project, not sure if with ViewModel approach or by passing around NSManagedObjectID (which is also a good idea for a architecture that supports well CoreData multithreading @jessesquires ;). Sometimes it is easy to forget not to pass a coredata object around, and end up with changing thread in the subsequent classes)

I will probably write a repository with examples and "tutorial" for IGListKit later on.
Maybe it can be used/merged into IGListKit FAQ #405 or into the documentation later on?

[rnystrom]

@racer1988

Maybe it can be used/merged into IGListKit FAQ #405 or into the documentation later on?

You bet! That's where I was leading 😄 It would be really really helpful. If you're feeling good should we close this?

@jessesquires's idea sounds awesome. Let us know if this works out! Might be a worthy example project too?

CoreData is beautiful, but misunderstood.

😅

[rnystrom]

@racer1988 are we ok to close this issue?

[pappalar]

Hi @rnystrom, yea it is ok, sorry I was away

[pappalar]

Hello @rnystrom, @jessesquires

I implemented the ViewModel etc.. and I got more issues than before, so
after spending more time investigating I noticed with @michalkalinowski- that the issue with the updating of existing values is present even without using CoreData.

We worked on your code here https://github.com/racer1988/IGListKitPerformUpdates

Investigation:
After booting the app, we add a message to the datasource, then we edit it.
The update is not done.

We thought is due to the objects not been copied, so we implemented NSCopying protocol in the model, but the copy method is never called by IGListKit.

We checked IGListKit code and we noticed that perform updates is not making a shallow copy to compare the objects, it is taking the map and the objects from the datasource and using those to make all the comparisons.
The problem seems to be that the IGListSectionMap and the datasource objects are never copied, so they are not triggering copy for the objects they contain.

Instead, Making a shallow copy of the map triggers the NSCopying protocol method on all the child objects, and makes the collection view update correctly the cells that have been edited.

I opened a PR for the same reason, can you please let me know if what we are doing makes sense, and if not, how would you fix the example project to achieve the update of the UI. We might have not understood how IGListKit expects to be used to achieve updates.

We will try next to use the "copy" fix to see if we can custom implement NSCopying on our CoreData Objects to see if we can use that method to replace old objects with new objects

[rnystrom]

@racer1988 what if in objectsForListAdapter: you call the initWithArray:copyItems: method before returning your objects?

[pappalar]

@rnystrom Yea, good idea, didn't think about moving it out of the framework lol :D

However even this approach has a issue. (even in the PR)
If i copy the whole array everytime, I get a array of new objects everytime.
This makes the collectionView flicker all the time perfomUpdates is called.

So going back to the coreData case:
Let's say I iterate over every object in my collection view, fetch the update from the server, update the object, save the context of the object to fix the update. (because every object uses its own API so can't batch them)

This triggers a NSFetchedResultControllerDelegate per object,
which in turns triggers a performUpdates per object.

If i have let's say 100 objects, I get the collectionView object to potentially flicker 100 times.

The objects are new, so IGListKit updates the collectionView by doing delete and insert on all of them.

My issue remains (or it become a user experience issue?), I probably did not understood the framework:
What is the way to use IGListKit, to show data for a collection view and having it updates in UI based on the changes to the model, with animation and no flickering due to object replacement?

(I am going to try Notification and SectionController refresh next)

[pappalar]

We'll try to implement a better diffing on our side to see if that solves the issue with ViewModel.
If that works, I'll work on the FAQ for CoreData with ViewModel

[pappalar]

I manage to make the update work on the example project, by implementing a specific diffIdentifier and isEqual method for the model and adding the copying mechanism on the datasource.

I guess that it would be nice to add in the IGListKit documentation that the framework is working out of the box only for immutable datamodels that are recalculated every time there is a perform updates.

Else if the model is mutable or it is not recalculated, it is necessary to implement NSCopying a create a copy of it in the datasource, in order to get the updates in UI.

This is not clear imho in the getting started guide and could bring to misleading development

[jessesquires]

Thanks @racer1988 !

We can update the "Getting Started" guide to discuss mutability.

It sounds like having a new, dedicated guide for CoreData would be good too. "Working with Core Data".

https://github.com/Instagram/IGListKit/tree/master/Guides

cc @rnystrom

[pappalar]

Yep, I'll draft something next week, so I can open a PR and we can contribute to it

[jessesquires]

Closing as resolved via #515