magicalpanda / MagicalRecord

Super Awesome Easy Fetching for Core Data!
Other
10.8k stars 1.79k forks source link

NSFetchedResultControllerDelegate && wrong context? #868

Open CeccoCQ opened 9 years ago

CeccoCQ commented 9 years ago

I've a big refresh problem in my app, in particular I follow these simple steps:

  1. Fetch data from server;
  2. Store data with MagicalRecord;

I expect that the a View with UITableView will be refreshed with new added records. But this not happens, I think that there is a problem with Context objects, because if I use the [NSManagedObjectContext contextForCurrentThread] all works correctly. But this is not recommended by Magical Record docs.

I have attached my code below with small description.

I've a UITableView with following code in lifecycle:

- (void)viewDidLoad {
    [super viewDidLoad];
    [self getFriends];    
}

-(void) viewWillAppear:(BOOL)animated{
    self.fetchedResultsController = [SPUserModel fetchAllSortedBy:@"fullName" ascending:YES withPredicate:nil groupBy:nil delegate:self];
    [self updatePredicate];
}

- (void)viewWillDisappear:(BOOL)animated {
    [super viewWillDisappear:animated];
    self.fetchedResultsController.delegate = nil;
}

and in NSFetchedResultControllerDelegate:

- (void)controllerWillChangeContent:(NSFetchedResultsController *)controller {
    [self.tableView beginUpdates];
}

- (void)controller:(NSFetchedResultsController *)controller didChangeObject:(id)anObject atIndexPath:(NSIndexPath *)indexPath forChangeType:(NSFetchedResultsChangeType)type newIndexPath:(NSIndexPath *)newIndexPath {
    UITableView *tableView = self.tableView;

    switch(type) {
        case NSFetchedResultsChangeInsert:
            [tableView insertRowsAtIndexPaths:[NSArray arrayWithObject:newIndexPath] withRowAnimation:UITableViewRowAnimationFade];
            break;

        case NSFetchedResultsChangeDelete:
            [tableView deleteRowsAtIndexPaths:[NSArray arrayWithObject:indexPath] withRowAnimation:UITableViewRowAnimationFade];
            break;

        case NSFetchedResultsChangeUpdate:
            [tableView reloadRowsAtIndexPaths:[NSArray arrayWithObject:indexPath] withRowAnimation:UITableViewRowAnimationNone];
            break;

        case NSFetchedResultsChangeMove:
            [tableView deleteRowsAtIndexPaths:[NSArray arrayWithObject:indexPath] withRowAnimation:UITableViewRowAnimationFade];
            [tableView insertRowsAtIndexPaths:[NSArray arrayWithObject:newIndexPath] withRowAnimation:UITableViewRowAnimationFade];
            break;
    }
}

- (void)controller:(NSFetchedResultsController *)controller didChangeSection:(id <NSFetchedResultsSectionInfo>)sectionInfo atIndex:(NSUInteger)sectionIndex forChangeType:(NSFetchedResultsChangeType)type {
    switch(type) {
        case NSFetchedResultsChangeInsert:
            [self.tableView insertSections:[NSIndexSet indexSetWithIndex:sectionIndex] withRowAnimation:UITableViewRowAnimationFade];
            break;

        case NSFetchedResultsChangeDelete:
            [self.tableView deleteSections:[NSIndexSet indexSetWithIndex:sectionIndex] withRowAnimation:UITableViewRowAnimationFade];
            break;

        case NSFetchedResultsChangeMove:
            break;

        case NSFetchedResultsChangeUpdate:
            break;
    }
}

- (void)controllerDidChangeContent:(NSFetchedResultsController *)controller {
    [self.tableView endUpdates];
}

The method [self updatePredicate] where I try to fetch all my friends:

- (void) updatePredicate {
    NSPredicate *messageFilter = [NSPredicate predicateWithFormat:@"isFriend != nil && isFriend == %@ && isMe == %@", [NSNumber numberWithBool:YES], [NSNumber numberWithBool:NO]];
    if(_searchText != nil) {
        NSString *query = @"isMe == %@ && isFriend != nil && isFriend == %@ && fullName CONTAINS[cd] %@";
        messageFilter = [NSPredicate predicateWithFormat: query, [NSNumber numberWithBool:NO], _searchText];
    }
    self.fetchedResultsController.fetchRequest.predicate = messageFilter;
    self.fetchedResultsController.delegate = self;
    [self.fetchedResultsController performFetch:nil];
    [self.tableView reloadData];
}

The method [self getFriends] :

- (void) getFriends {
    PMKPromise *getFriends = [self.userManager getFriendsWithPage:0 withLimit:1000];
    getFriends.then(^(OVCResponse *response) {
        [selfstoreFriends:response.result];
    }).catch(^(NSError *error) {
        ScxResponseDTO * dict = [SPBaseManager responseFromError:error];
        DDLogError(@"%@ %@", dict.code, dict.message);
        [super handleInvalidAccessToken:dict];
    }).finally(^{
        [self.tableView.pullToRefreshView stopAnimating];
    });
}

The method [self storeFriends]:

- (void) storeFriends:(NSArray *)friends {

    [MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {

        // ---- If I uncomment this line, the app works fine -----
        //localContext = [NSManagedObjectContext contextForCurrentThread];

        for(ScxUserDTO *userDTO in friends) {

            SPUserModel *userDB = [SPUserModel findFirstByAttribute:@"uid" withValue:userDTO.uid inContext:localContext];
            user.isFriend = YES;
            user.friendshipId = userDTO.friendship.identifier;
            user.frienshipStatus = userDTO.friendship.status;

        }

        [localContext saveToPersistentStoreAndWait];
    }];
}

Why?!?!?!?!? Please help me.

tonyarnold commented 9 years ago

Yeah, definitely don't use contextForCurrentThread — it has been removed in MR3.

Two things:

  1. Can you show me how you're setting up MagicalRecord?
  2. You don't need to save localContext — remove [localContext saveToPersistentStoreAndWait]; from your storeFriends: method
CeccoCQ commented 9 years ago

Hi Tony, thanks for your response. I don't want to use the contextForCurrentThread, but I can't find any solution for my problem.

Let me answer for point:

  1. Simply invoke [MagicalRecord setupCoreDataStackWithAutoMigratingSqliteStoreNamed:@"Main"]; in my AppDelegate.m file;
  2. But if I remove this method, when Magical Record saves insert/update/delete operations in database?
tonyarnold commented 9 years ago

Thanks for the info. So [MagicalRecord saveWithBlock:…]; lets you make changes to localContext, which are then saved back to your default context. You don't need to call save on localContext yourself, we do that for you once your block has been executed.

CeccoCQ commented 9 years ago

I've removed the [localContext saveToPersistentStoreAndWait] line but still not works :(

CeccoCQ commented 9 years ago

News.

I've removed [self getFriends] from viewDidLoad and I've moved this method in viewDidLoad with Truncate method (only for testing).

- (void)viewDidAppear:(BOOL)animated {
    [MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
        [SPUserModel truncateAllInContext:localContext];
    } completion:^(BOOL success, NSError *error) {
        [self getFriends];
    }];
}

all works correctly. O.O

Another info: if I invoke [SPUserModel truncateAllInContext:localContext]; on viewDidLoad the app crashes.

Why???

Alydyn commented 9 years ago

Where are you setting self.context, and what are you setting it to?

CeccoCQ commented 9 years ago

Sorry, the self.context is localContext.

This is a copy&paste problem.

Alydyn commented 9 years ago

So does it crash or not when you call -truncateAllInContext:? It sounds like you are saying both...

CeccoCQ commented 9 years ago

Hi Alydyn,

If I invoke -truncateAllInContext within viewDidLoad the app crashes:

2014-10-02 14:37:04.134 SPapp[57195:922753] *** Terminating app due to uncaught exception 'NSObjectInaccessibleException', reason: 'CoreData could not fulfill a fault for '0xd000000003980000 <x-coredata://AD716A5B-C11D-46B7-BDDD-739E2FA208E1/SPUserModel/p230>''

If I move -truncateAllInContext within viewDidAppear all works correctly also without the localContext = [NSManagedObjectContext contextForCurrentThread]; statement (read my first post).

Alydyn commented 9 years ago

@CeccoCQ can you comment out your call to [self getFriends] in the completion and see if it still crashes?

CeccoCQ commented 9 years ago

If I remove [self getFriends] method, the app still crashes. I've missed another info, the table User has a relationship (below the Entity diagram)

schermata 2014-10-02 alle 14 58 29

So I've changed the method within viewDidLoad:

[MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
    [SPReservationModel truncateAllInContext:localContext];
    [SPUserModel truncateAllInContext:localContext];
    [SPContentModel truncateAllInContext:localContext];
    [SPEventModel truncateAllInContext:localContext];
} completion:^(BOOL success, NSError *error) {
    //[self getFriends];
}];

but the app works only within viewDidAppear.

The think that I don't undestand is because the refreshing of TableView after a HTTP Request works correctly only if I use the truncate method at viewDidAppear...

Alydyn commented 9 years ago

It sounds to me like something isn't completely set up when you are calling -viewDidLoad, and has had a chance to finish by the time -viewWillAppear: is called. Maybe @tonyarnold has more info?

CeccoCQ commented 9 years ago

I also think so. But why the controllerWillChangeContent is called only if I execute a truncate operation or only if into storeFriends I use [NSManagedObjectContext contextForCurrentThread] ??

Another test. I've added this method:

 - (IBAction)didChangeSomething:(id)sender {
        [MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
        SPUserModel *userDB = [SPUserModel findFirstByAttribute:@"uid" withValue:@"1234567890abc" inContext:localContext];
            userDB.isFriend = [userDB.isFriend boolValue] ? [NSNumber numberWithBool:NO] : [NSNumber numberWithBool:YES];
            [localContext saveToPersistentStoreAndWait];
        }];
    }

But nothing happens :(

Alydyn commented 9 years ago

Again, you do not need to call -saveToPersistentStoreAndWait. MagicalRecord calls that for you after the block completes.

And also, try using +MR_fetchAllSortedBy:(NSString *)sortTerm ascending:(BOOL)ascending withPredicate:(NSPredicate *)searchTerm groupBy:(NSString *)groupingKeyPath inContext:(NSManagedObjectContext *)context and specifying [NSManagedObjectContext MR_defaultContext].

I just checked the method implementation, and I think that the problem you are seeing is that the variants of that method that do NOT specify a context are using [NSManagedObjectContext MR_contextForCurrentThread]. @tonyarnold I assume that should be changed?

CeccoCQ commented 9 years ago

I've checked, if I remove -saveToPersistentStoreAndWait method, nothing works. Looking into database the value for isFriend remains always to 0.

Alydyn commented 9 years ago

Make sure you set up your fetched results controller with the default context like I mentioned above. Also, what version of MagicalRecord are you using?

CeccoCQ commented 9 years ago

I've changed with: self.fetchedResultsController = [SPUserModel fetchAllSortedBy:@"fullName" ascending:YES withPredicate:nil groupBy:nil delegate:self inContext:[NSManagedObjectContext MR_defaultContext]]; But nothing.

I'm using pod 'MagicalRecord/Shorthand'

Alydyn commented 9 years ago

I believe that pulls version 2.2. I suggest you pull it from Cocoapods and get the latest beta of version 2.3

CeccoCQ commented 9 years ago

I've tried but on cocoapods there is only version 2.2. Is there a way to pull the 2.3 version?

Alydyn commented 9 years ago

I am not sure, I try not to use Cocoapods. I would remove it from your pod file, re-run pod install, and then download and manually import version 2.3

waltermvp commented 9 years ago

Try this

pod "MagicalRecord", :git => 'https://github.com/magicalpanda/MagicalRecord.git', :branch => 'develop'
CeccoCQ commented 9 years ago

@waltermvp thanks! @Alydyn bad news, also with version 2.3 I've same behaviour.

waltermvp commented 9 years ago

Is the issue that savewithblock's completion block is not getting called?

Alydyn commented 9 years ago

The main issue is that his NSFetchedResultsController is not updating his tableView.

@CeccoCQ in the completion block call [self.fetchedResultsController performFetch:nil].

waltermvp commented 9 years ago

That should work. Keep in mind that the delegate updates get called when data in the moc changes. It's easy to expect a update when the predicate changed but that's not intended. Changing the predicate will not call the delegates you must perform fetch anytime you set the predicate. One nice way would be to subclass override the setpredicate method of that fetch request and call Perform fetch there.

CeccoCQ commented 9 years ago

IMHO I think that the context was wrong. Is it possible that if I change the localContext returned from saveWithBlock is different from the context used in fetchAllSortedBy... ??

I think that because if I set localContext with contextForCurrentThread all works.

waltermvp commented 9 years ago

Shouldn't matter the save with block context should save all the way to the persistent store. Which is probably being observed from your main thread context

CeccoCQ commented 9 years ago

I'm also thinking so but still remain the problem of use of contextForCurrentThread and why this works...

Alydyn commented 9 years ago

@CeccoCQ When you call +saveWithBlock:, MagicalRecord creates a new child context with MR_rootSavingContext as the parent. MR_defaultContext is already configured to listen for save notifications from MR_rootSavingContext, but NSFetchedResultsController uses the notification generated by -processPendingChanges.

Interestingly I just noticed that +MR_fetchAllSortedBy:(NSString *)sortTerm ascending:(BOOL)ascending withPredicate:(NSPredicate *)searchTerm groupBy:(NSString *)groupingKeyPath inContext:(NSManagedObjectContext *)context actually ignores the passed in context. It uses it to create the fetch request but still uses +MR_contextForCurrentThread when it creates the NSFetchedResultsController.

waltermvp commented 9 years ago

Contextforcurrentthread "may work" but is not guaranteed, I believe

CeccoCQ commented 9 years ago

@waltermvp This is my fear... @Alydyn I've a news, this is a simple code that I added today:

- (IBAction)didChangeSomething:(id)sender {
    [MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
          SPUserModel *userDB = [SPUserModel findFirstByAttribute:@"uid" withValue:@"a3535ee6-645a-4f6e-9970-4df65120353e" inContext:localContext];
         userDB.isFriend = [userDB.isFriend boolValue] ? [NSNumber numberWithBool:NO] : [NSNumber numberWithBool:YES];
        [localContext saveToPersistentStoreAndWait];
    }];
}

and nothing happens on the TableView, but into the database the value of isFriend is correctly updated. Now, if I change this code with:

- (IBAction)didChangeSomething:(id)sender {
     SPUserModel *userDB = [SPUserModel findFirstByAttribute:@"uid" withValue:@"a3535ee6-645a-4f6e-9970-4df65120353e"];
    [MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
        [userDB inContext:localContext];
          userDB.isFriend = [userDB.isFriend boolValue] ? [NSNumber numberWithBool:NO] : [NSNumber numberWithBool:YES];
        [localContext saveToPersistentStoreAndWait];
    }];
}

then works!!!!! For "works" I mean that the TableView show and hide the record.

Bah....

Alydyn commented 9 years ago

And you still need to call [localContext saveToPersistentStoreAndWait] to get it to work? Have you tried commenting that out?

CeccoCQ commented 9 years ago

@Alydyn If I comment this line, nothing works... the app doesn't update the database field. Anyway I've already tried to delete saveToPersistentStoreAndWait but with no luck.

waltermvp commented 9 years ago

I see what you mean now. I've run into the same issue. For some reason unknown to me, fetching from the main thread then fetching the same object using in context from the local context works. It should work in both scenarios but does not. Dunno why

CeccoCQ commented 9 years ago

Stupid question...

Is it possible that I have these problems because I execute an insert/update entity out of ViewController? In my example, storeFriends was executed in a different class....

Alydyn commented 9 years ago

It's very hard to say without seeing all of the code. When debugging issues like this I like to eliminate any variables that I can. Certainly, reducing the logic to the minimum necessary would help troubleshoot what is happening.

If your main issue is that the NSFetchedResultsController does not automatically refresh with your current implementation then I think you need to create the simplest possible scenario where you can reproduce it. Then you can place multiple breakpoints and see what is being called and when. You are dealing with multiple contexts running in multiple threads so it is possible that things are not happening when you think they are.

CeccoCQ commented 9 years ago

I have some news. If I add the completion block, all works correctly (see code below). I'm wondering, if I execute a new [self.fetchedResultsController performFetch:nil];, could I have some problems when user scrolls the list or could I have some freezes within tableview?

- (void) getFriends {
     PMKPromise *getFriends = [self.userManager getFriendsWithPage:0 withLimit:1000];
     getFriends.then(^(OVCResponse *response) {
        [self.userManager storeFriends:response.result completion:^(BOOL success, NSError *error) {
            [self updatePredicate];
        }];
     }).catch(^(NSError *error) {
    }).finally(^{
        [self.tableView.pullToRefreshView stopAnimating];
    });
}
tonyarnold commented 9 years ago

@CeccoCQ yes, calling performFetch: could cause stutters or small pauses in your UI.