NSManagedObject changes not detected by TLIndexPathController

[davetroy]

Hi there. I am using the latest TLIndexPathTools (0.4.3) with Core Data and came across a bug that seems to be related to TLIndexPathController (or my understanding of it.)

In short, TLIndexPathController is not populating modifiedItems when it should be. It does call controller: didUpdateDataModel but updates has an empty modifiedItems array.

Not being sure where the issue was, I wrote two tests, one against NSFetchedResultsController and another against TLIndexPathController to detect the same exact changes. The NSFetchedResultsController tests work as expected, while the TLIndexPathController tests fail to detect intermediate changes. I would like to have the NSFetchedResultsController behavior.

I think this is either a serious issue, or a wild misunderstanding on my part. Either way I'd appreciate your guidance, and if it is an issue, I'd love to try to help resolve it. This library is a godsend and besides for this issue has been 100% solid for me. Thanks.

My tests:

• FRCTests.swift
• TLToolsTests.swift

[davetroy]

Hm, OK. :) Digging further into your code, I guess what I was missing here was the modificationComparatorBlock option. It seems isEqual: is insufficient to detect Core Data property changes. With a more detailed comparator block set, I can detect the changes I'm looking for.

So, I guess this raises two questions. 1) Is my diagnosis correct, and do you think that's the best way to address the issue?, and 2) If isEqual doesn't detect basic property changes, what does it detect, and what should it detect?

Thanks for any ideas you may have on this. I certainly learned something today.

[wtmoose]

Detecting modifications is kind of a weak point of the framework :)

As you can guess, isEqual only works if the data models being compared have different instances. However, TLIndexPathController has a feature to detect updated Core Data objects: see references to the updatedItems property in TLIndexPathController. The modificationComparatorBlock was added recently by a contributor to help with non-Core Data objects, so you don't need to use that.

I could probably spot the problem right away if I had a working project for your test case. I'd suggest adding some breakpoints in TLIndexPathController to find out what's going on with updatedItems. My only guess so far is that controllerWillChangeContent is getting called a second time before controllerDidChangeContent since that is where updatedItems gets cleared.

[davetroy]

Thanks for the input. The best guess I have right now is that isEqual: just doesn't detect the property changes. So, it's deciding it's the same object (which it is), even though property changes have occurred. If, in my modificationComparatorBlock I explicitly check a couple of properties I care about, then it seems to work perfectly.

I need to dig into this a bit deeper and see how it can be addressed most simply. Tomorrow. :)

[wtmoose]

There should only ever be one instance of any given Core Data object in a context. So, isEqual will never work as it’s comparing to itself. That is why TLIndexPathController detects updated Core Data objects for you internally (using an NSFetchedResultsController). It should just work. No need to worry about isEqual or modificationComparatorBlock. If it doesn’t work then, I would suggest we do a bit of digging to determine why.

On May 15, 2016, at 6:20 PM, Dave Troy notifications@github.com wrote:

Thanks for the input. The best guess I have right now is that isEqual: just doesn't detect the property changes. So, it's deciding it's the same object (which it is), even though property changes have occurred. If, in my modificationComparatorBlock I explicitly check a couple of properties I care about, then it seems to work perfectly.

I need to dig into this a bit deeper and see how it can be addressed most simply. Tomorrow. :)

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/wtmoose/TLIndexPathTools/issues/68#issuecomment-219319000

[davetroy]

Hm, yeah. That's sort of what I figured initially. If that's the case, I can only guess there is something wrong with the plumbing in how NSFetchedResultsController is supplying into to TLIndexPathController, as it's clear the former works when used alone. Tomorrow I will see about putting together a simple standalone project to try to isolate this.

[davetroy]

I just did a quick look to see if willUpdateDataModel is being called more than once. So far that does not look to be the case, so it seems more a case that updatedItems is not being consistently populated rather than being inadvertently cleared.

[davetroy]

OK, attached is a sample project that exactly illustrates the issue I am having. I've included tests against NSFetchedResultsController which all work fine, and tests against TLIndexPathController, which fail. You can make the TLIndexPathController tests pass by setting the comparator block, but as you say, that should be unnecessary, and I've discovered that in practice it updates way too much stuff and bogs down the app. Any thoughts and/or fixes appreciated! Hope this helps!

TLSample.zip

Update: weirdly, the main issue seems to be that controller: didChangeObject: is not being called consistently. Even though in my FRC tests it is relying on the exact same thing, it simply doesn't seem to be called on each change when running inside TLIndexPathController. Seems like a very weird bug.

Another update: I think I found the bug. In addition to tracking .Change updates, TLIndexPathController needs to track .Move updates. Even though some moves are in-place, I think they will get reported that way if a key that may affect the sort order has been modified. Not sure if there are other side effects that need to be considered here but this change to controller: didChangeObject: makes tests pass.

    if (type==NSFetchedResultsChangeMove && indexPath == newIndexPath) {
        [self.updatedItems addObject:anObject];
    }

Let me know what you think!

[wtmoose]

The main issue with the text is that NSFetchedResultsController reports changed message count as a move, rather than an update, because you're sorting on messageCount. The test passes if you set the sort descriptor to an empty array.

You'll also want to use permanent object IDs so that TLIPS can keep track newly created objects saves. To do this in your test, for example, add the following following line after creating the address:

try! mainContext.obtainPermanentIDsForObjects([address])

Alternatively, if your model object has an ID field of its own, you can pass that as the identifierKeyPath in the TLIndexPathController constructor.

Does that help?

[davetroy]

Ah, ok, thanks. I will look into this. Yeah, I do need to sort on messageCount. So, to me the current behavior is a bug and it seems this fix is needed. Will you be putting it in? Because it's not working in production otherwise.

I will look at the .obtainPermanentIDsForObjects and/or consider passing in another identifier. Thanks for the help!

[wtmoose]

TLIPS seems to be consistent with NSFetchedResultsController except in the case where you make a change affecting the sort column, but not the actual sort order, wherein NSFetchedResultsController reports zero-distance moves. I could add a workaround in TLIPT to also reports zero-distance moves. Is that what you had in mind?

[davetroy]

Yeah. I think that would be necessary for anyone wanting to use TLIPT with a sort order specified, and properly report such zero-distance moves. There doesn't seem to be any other way to catch it and I absolutely need that for my app.

[wtmoose]

Got this on my todo list.

[davetroy]

Thanks. I am using my hack in the meantime and it seems to be pretty solid.

[davetroy]

One other note, in case it is relevant. I am seeing issues where when a predicate is set on the TLIndexPathController fetchRequest, sometimes .insertedItems is not populated with an entity if that item previously existed but was newly changed to meet the predicate condition. Forcing a .performFetch() seems to fix it.

To illustrate, if fetchRequest.predicate = NSPredicate(format: "count>0"), then,

object.count=0 // object is not shown
object.count=5 // may not be shown until performFetch()

Thanks!

[wtmoose]

Can you check if TLIndexPathController.controllerDidChangeContent(_) is getting called? If it is, I can't see how TLIPT wouldn't do the right thing, so I'll need to reproduce it on my end.

[davetroy]

Actually, scratch that last issue. I am pretty sure I figured out what was causing it, and I think it's something I did. Original issue still stands. Thanks so much!

[davetroy]

Any update on this? One thing I am still seeing, even with my 'fix' in place — let's say object.count on a cell, goes from 1 to 4 and the sort order is on that field.

1. The cell is moved
2. The object is not refreshed on that cell

Therefore, an indicator displaying object.count will remain at 1 and not be refreshed to 4. I am pretty sure this is a TLIPT issue related to the move operation.

[wtmoose]

It isn't a TLIPT issue. Table view's don't call cellForRowAtIndexPath when you do a move operation. So, the cell content doesn't get updated. Typically, what I do is put my cell configuration logic in TLTableViewController.tableView(:configureCell:atIndexPath) and call TLTableViewController.reconfigureVisibleCells() in didUpdateDataModel.

[davetroy]

Ah, thanks. This is on a UICollectionView but I imagine the logic is basically the same. I will check that out!