Duplicates with saveWithBlock

[laurentcabon]

Hello,

Sorry for my english.

My problem comes when I want to parse an array of objects from a server by checking that I do not get an object already added to my database. Indeed :

1 call (saveWithBlock+check+parsing) -> it works x call at the same time or delayed (saveWithBlock+check+parsing) -> I get duplicates randomly

Here is my code :

[MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
        NSArray *serverResponse;
        for(NSDictionary *dic in serverResponse){
            NSPredicate *predicate = [NSPredicate predicateWithFormat:@"idToto = %@", dic[@"id"]];

            if([OFToto MR_countOfEntitiesWithPredicate:predicate inContext:localContext] == 0){
                // Create Object
                OFToto *toto = [OFToto MR_createInContext:localContext];
                toto.idToto = dic[@"id"];
            }
            else {
                // DO NOTHING, object already added
            }
        }
    } completion:^(BOOL success, NSError *error) {
        // UPDATE UI
    }];

Did I do something wrong ?

Thanks

[waltermvp]

Try this pattern to see if it helps

        [MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
            NSArray *messages = arrayFromNetwork;
            [messages enumerateObjectsUsingBlock:^(id obj, NSUInteger idx, BOOL *stop) {
            Publisher *user = [Publisher MR_findFirstByAttribute:@"identifier" withValue:userID inContext:localContext];
            if (!user) {
                user = [Publisher MR_createEntityInContext:localContext];
            }
             user.identifier = userID;
            }];
        } completion:^(BOOL contextDidSave, NSError *error) {
            if (thisBlock) {
                thisBlock(error);
            }
        }];
    } failure:^(AFHTTPRequestOperation *operation, NSError *error) {
        if (block) {
            block(error);
        }
    }];

[laurentcabon]

Hello,

Thanks, but it doesn't work, there are always duplicates..

Another idea ?

[waltermvp]

@laurentcabon that's weird are you sure the idToto attribute it unique?

[laurentcabon]

yes, idToto received by the server is unique , the bug occurs when I do multiple queries + parsing in several view controllers in a page view controller .

For example :

PageViewController (filtered content for vc1/vc2/vc3/vc4 but same objects) ViewController1 => query + parsing (viewDidAppear) ViewController2 => query + parsing (viewDidAppear) ViewController3 => query + parsing (viewDidAppear) ViewController4 => query + parsing (viewDidAppear)

=> Random duplicates :s

[waltermvp]

@laurentcabon i see. This should not be occurring, are you always using the localContext for both fetching creating and populating?

[laurentcabon]

Yes, i'm always using the localContext, it's very strange. It appears only when I use "[MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext)". If I don't save data, the problem doesn't occur but it freeze UI (not in a background thread..). Very strange !

[waltermvp]

Try using + (void) saveWithBlock:(void(^)(NSManagedObjectContext *localContext))block completion:(MRSaveCompletionHandler)completion; and return the block when the completion block gets called. What are you using for networking?

[laurentcabon]

I don't understand, how could I return the block ? I use AFNetworking for networking.. Thanks

[ayvaroff]

Hi. I got the same problem. And after my app did enter foreground and reappear there created another thread that handles saveWithBlock. My guess that's the problem. And I still can't understand why app creates as many threads as I do the operation above.

[waltermvp]

@laurentcabon something like below. Call the completion block of your own method in the completion block of the Magical record save call

- (AFHTTPRequestOperation *)updateGroupsWithBlock:(void (^)(NSError *))block {
    NSDictionary *params = @{@"userAlias":self.alias};
    AFHTTPRequestOperation *operation = [[MyWorldRestAPI sharedManager] POST:@"GetUserGroups" parameters:params success:^(AFHTTPRequestOperation *operation, id responseObject) {
        NSArray *groupsDict = responseObject[@"d"];
        [MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
            NSMutableArray *freshGroups = [[NSMutableArray alloc] init];
            [groupsDict enumerateObjectsUsingBlock:^(id obj, NSUInteger idx, BOOL *stop) {
                NSDictionary *topLevel = obj;
                Publisher *group = [Publisher publisherWithJSON:topLevel[@"GroupPublisher"]
                                        ignoreUnavailableValues:NO
                                                      inContext:localContext];
                group.type = [NSNumber numberWithInteger:PublisherTypeGroup];
            }];
        } completion:^(BOOL contextDidSave, NSError *error) {
            if (block) {
                block(error);
            }
        }];
    } failure:^(AFHTTPRequestOperation *operation, NSError *error) {
        if (block) {
            block(error);
            NSLog(@"MyWorldRestAPI getGroupWithAlias line 106: %@",[error.userInfo objectForKey:NSLocalizedDescriptionKey]);
        }
    }];
    return operation;
}

[laurentcabon]

It does not work :s Same issue..

[waltermvp]

@laurentcabon pretty stomped. how did you import magical record into your project?

[laurentcabon]

Hello and Happy New year !

I use Cocoapods to import magical record..

[waltermvp]

can you try removing the Magical record pod, deleting the podfile and running pod install again?

[laurentcabon]

I just do what you tell me and I have the same behavior. I think there is a bug in MagicalRecord with thread parallel execution ..

[waltermvp]

@laurentcabon there may be. @tonyarnold any ideas?

[laurentcabon]

Any idea ?

[tonyarnold]

I'm not aware of any threading bugs in either of the releases under development. There might be in the released 2.2 version, but if there were I would expect many more users to be reporting the problem.

My guess is that your predicate isn't working the way you expect it to. Have you verified that it's matching any existing managed objects?

[laurentcabon]

Yes, my predicate works in most of the case, but randomly it does not work.. in particular when I make many requests (+ parsing) at the same time..

[ahknight]

I had this problem in my project and it's kind of funny what happened. I was hitting about 10 different API endpoints (full sync) with AFNetworking and doing the saves in the AFNetworking complete block. Each one started a new localContext at a different point in time than the others (think transactions) so some saw objects in the main context and some didn't because all were saving at the same time.

The solution (for me) was to create an NSOperationQueue of width 1 and enqueue all save requests with addBlock: and use MR's blocking save in that block. This serialized the saves and ensured that when localContext was created it was using the results of the previous API call's save.

Took a while to figure out why I had empty objects related to some objects even though the relation existed. The object simply wasn't visible in the child context yet due to timing. You'd have the same problem with a "big" database and transactions, too.

The issue, in greater detail, is that context B has created the real object X from its import because it didn't find one yet, context A didn't find it at the time it set a relation (because context B hasn't saved yet) and so it created object Y and related to that, instead. Both A and B save and now you have duplicates and ghosts. Instead, by assuring that context A saves before B, B will find object X and update/relate to it instead.

[laurentcabon]

Hello and Thanks. What do you mean with "and use MR's blocking save in that block" ? I don't understand..

[ahknight]

There's a method on MagicalRecord called, IIRC, -saveAndWait that blocks the thread until completion. Enqueue a block on the operation queue that uses that kind of save so you can't have more than one import/save in-flight at once.

[waltermvp]

@ahknight do you mind sharing your implementation for queuing magical saves? Curios how your going about it.

[ahknight]

Here's a very minimal version. I kept a similar method signature to MR's async save method so it would be a drop-in replacement for the most part.

@interface MyClass : NSObject

/// Used to serialize Core Data operations.
@property (nonatomic) NSOperationQueue *saveQueue;

// Save Operation Management
-(void)enqueueSaveOperationWithBlock:(void(^)(NSManagedObjectContext *localContext))saveBlock completion:(MRSaveCompletionHandler)completion;

@end

@implementation MyClass

-(instancetype)init
{
    self = [super init];
    if (self) {
        self.saveQueue = [NSOperationQueue new];
        _saveQueue.maxConcurrentOperationCount = 1;
        _saveQueue.qualityOfService = NSOperationQualityOfServiceUtility;
  }
  return self;
}

#pragma mark - Save Operation Management

-(void)enqueueSaveOperationWithBlock:(void(^)(NSManagedObjectContext *localContext))saveBlock completion:(MRSaveCompletionHandler)completion
{
    [self.saveQueue addOperationWithBlock:^{
        [MagicalRecord saveWithBlockAndWait:saveBlock];
        if (completion != nil) {
            dispatch_sync(dispatch_get_main_queue(), ^{
                completion(YES, nil);
            });
        }
    }];
}

-(void)someMethod
{
    [self enqueueSaveOperationWithBlock:^(NSManagedObjectContext *localContext) {
        [object setValue:@"foo" forKey:@"bar"];
    } completion:^(BOOL contextDidSave, NSError *error){
        NSLog(@"All done!");
    }];
}

@end

[waltermvp]

@ahknight thanks its looks like a good solution. @tonyarnold do you see functionality like this ever making it to MR?

[laurentcabon]

Ok, it works but there is a problem. It seems that the saves are not done in background.. It freeze UI..

[bachle]

@laurentcabon Did you ever solve this problem? Everytime I use saveWithBlock, it results in duplicates even though the relatedByAttribute values are set. Happens in 2.2 and 2.3 beta5.

[mrmarcsmith]

I am having the same issue! I had to resort back to:

[[NSManagedObjectContext MR_defaultContext] MR_saveToPersistentStoreAndWait];

I would love to use saveWithBlockAndWait: but every time I quit the app and return it duplicates my data!

[tonyarnold]

@tonyarnold do you see functionality like this ever making it to MR?

There's no reason why it couldn't, but I haven't had much time to work on MR lately. If it's still something you think would be useful, I can certainly have a look for version 3.0.

[tonyarnold]

@mrmarcsmith reckon you could share the code in your app that appears to be duplicating?

[mrmarcsmith]

Sure! Background: I use parse.com as my backend. What happens is everything happens as i expect until I close out the app and reopen it. At that time there are 2,3 or up to 10 duplicates of my events! If I rerun the code below then it deletes all the copies until i close out again.

PFQuery *query = [PFQuery queryWithClassName:@"ClubEvents"];
query.limit = 1000;
//only grab the events that are the clubs im signed up for
NSArray *myClubsArray = [MyYLClass arrayOfMyClubsOnly];
[query whereKey:@"clubGroup" containedIn:myClubsArray];

//only grab events that haven't been running for 10 hours
[query whereKey:@"clubTimeKidsStart" greaterThan:[[NSDate date] dateByAddingTimeInterval:-(60*60*10)]];
[query orderByAscending:@"clubTimeKidsStart"];
[query findObjectsInBackgroundWithBlock:^(NSArray *allClubEvents, NSError *error) {
    if (!error) {

        //replace current database with parse
        [ClubEvents MR_truncateAll];

        for(PFObject *anEvent in allClubEvents){
            //add each of my apps to Magical Record
            [MagicalRecord saveWithBlockAndWait:^(NSManagedObjectContext *localContext) {
                ClubEvents *newEvent = [ClubEvents MR_createEntityInContext:localContext];

                newEvent.pfObjectID = anEvent.objectId;

                newEvent.clubName = [anEvent objectForKey:@"clubName"];
                newEvent.clubIsBeingEditedBy = [anEvent objectForKey:@"clubIsBeingEditedBy"];
                newEvent.clubCreatedByUsername = [anEvent objectForKey:@"clubCreatedByUsername"];
                newEvent.clubGroup = [anEvent objectForKey:@"clubGroup"];
                newEvent.clubDress = [anEvent objectForKey:@"clubDress"];
                PFFile *file = [anEvent objectForKey:@"clubPhoto"];
                newEvent.clubPhoto = [file getData];
                newEvent.clubLocationName = [anEvent objectForKey:@"clubLocationName"];
                newEvent.clubAddress = [anEvent objectForKey:@"clubAddress"];
                newEvent.clubLocationStreet = [anEvent objectForKey:@"clubLocationStreet"];
                newEvent.clubLocationCity = [anEvent objectForKey:@"clubLocationCity"];
                newEvent.clubLocationState = [anEvent objectForKey:@"clubLocationState"];
                newEvent.clubLocationZip = [anEvent objectForKey:@"clubLocationZip"];
                newEvent.clubNotes = [anEvent objectForKey:@"clubNotes"];
                newEvent.clubTimeKidsStart = [anEvent objectForKey:@"clubTimeKidsStart"];
                newEvent.clubTimeLeadersStart = [anEvent objectForKey:@"clubTimeLeadersStart"];
                newEvent.clubScheduleArray = [NSKeyedArchiver archivedDataWithRootObject:[anEvent objectForKey:@"clubScheduleArray"]];;
                newEvent.clubIsTenative = [anEvent objectForKey:@"clubIsTenative"];
                NSData *clubRolesInvitedData = [NSKeyedArchiver archivedDataWithRootObject:[anEvent objectForKey:@"clubRolesInvited"]];
                newEvent.clubRolesInvited = clubRolesInvitedData;
                NSData *clubEmailsNotAttendingData = [NSKeyedArchiver archivedDataWithRootObject:[anEvent objectForKey:@"clubEmailsNotAttending"]];
                NSData *clubEmailsAttendingData = [NSKeyedArchiver archivedDataWithRootObject:[anEvent objectForKey:@"clubEmailsAttending"]];

                newEvent.clubEmailsNotAttending = clubEmailsNotAttendingData;
                newEvent.clubEmailsAttending = clubEmailsAttendingData;

                newEvent.clubWantToBePremadeTemplate = [anEvent objectForKey:@"clubWantsToBePremadeTemplate"];

            }];

            [MyYLClass addEventWithPFObject:anEvent withCompletionHandler:nil];

        }

        NSUserDefaults *defaults = [NSUserDefaults standardUserDefaults];
        [defaults setObject:[NSDate date] forKey:@"dateOfLastDownload"];
        [defaults setBool:NO forKey:@"isBusyUpdating"];
        [[NSNotificationCenter defaultCenter] postNotificationName:@"downloadSucceeded" object:nil];

    } else {
       //Alert the user the download failed
    }

}];

[tonyarnold]

@mrmarcsmith, I can't see that you're doing any kind of de-duplication in there. Each time you run the Parse request, you're using -[ClubEvents MR_createEntityInContext:] without checking to see if the managed object already exists.

What I would recommend you try is replacing that line with:

ClubEvents *newEvent = [ClubEvents MR_findFirstOrCreateByAttribute:@"pfObjectID" withValue:anEvent.objectId inContext:localContext]; 

I assume that pfObjectID is a unique identifier attribute on your ClubEvents entity?

This will check if an event already exists — there's probably no harm in re-setting all of the values on the managed object, although it may become a performance issue if you're processing thousands of entries.

[mrmarcsmith]

@tonyarnold I appreciate your response, I tried your solution and it still resulted in duplicates! I check the entries after the completion of this code (via MR_FindAll) and it is exactly what I expect it to be (let's say 30 entities). I then immediately kill the app and the very first line of code that runs in my appDelegate after setUpAutoMohratingCoreDataStack is my MR_FindAll and it always shows 60,90, or 120 entities. With each entity duplicated 2,3 or 4 times. My theory after reading your post: i have limited knowledge on the subject but I wonder if this is an issue that has to do with the persistent store vs localContext? My question: when I do MR_FindAll does it look at the localContext or the persistent store? For example maybe the MR_FindAll grabs everything were [ClubEvents MR_TruncateAll] which is at the beginning of my code only deletes the Persistent store but not the localContext or vice versa? Sorry for the potentially stupid question!

[mrmarcsmith]

@tonyarnold Any idea on what's wrong?

[tonyarnold]

My theory after reading your post: i have limited knowledge on the subject but I wonder if this is an issue that has to do with the persistent store vs localContext? My question: when I do MR_FindAll does it look at the localContext or the persistent store?

If you use MR_findAll without specifying a context, it will use the default context. If you specify it, it will look at whatever context you pass to it.

For example maybe the MR_FindAll grabs everything were [ClubEvents MR_TruncateAll] which is at the beginning of my code only deletes the Persistent store but not the localContext or vice versa?

Do you save the context when using MR_truncateAll? Any action that changes information in the context needs to be saved for it to have an effect.

[sruizpe]

Same problem here using MR 3.0 and before with MR 2.2.

I suspect the problem is checking an inexistent identificationAttribute on different localcontexts at the same time. Both of them will create a new object in DB (the same) when the block have been completed.

It may happens if you are requesting different/the same endpoints, at the same time, with the same objects in return.

There is another ticket related to this: https://github.com/magicalpanda/MagicalRecord/issues/1014

- (void)storeUser:(User *)user completion:(BooleanCompletionBlock)completion
{
    [MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
        UserDB *userDB = [UserDB MR_findFirstByAttribute:[[UserDB identificationAttributes] firstObject]
                                               withValue:user.userGUID
                                               inContext:localContext];
        if (!userDB)
            userDB = [UserDB MR_createEntityInContext:localContext];

        [self _fillUserDB:&userDB withUser:user inContext:localContext];
    } completion:^(BOOL success, NSError *error) {
        if (completion) completion(success, error);
    }];
}

Any non-blocking workaround proposal?. I am stuck finding solutions.

[sruizpe]

I think I found a fully functional workaround using KVO. Inside my UserDB.m (NSManagedObject):

-(BOOL)validateUserGUID:(id *)ioValue error:(NSError * __autoreleasing *)outError
{
    // Validate primary key to be unique before save in DB. Different MR private threads might save at the same time.
    NSFetchRequest *fetchRequest = [NSFetchRequest fetchRequestWithEntityName:NSStringFromClass([self class])];
    fetchRequest.predicate = [NSPredicate predicateWithFormat:@"userGUID == %@ AND self != %@", *ioValue, self];

    int count = [self.managedObjectContext countForFetchRequest:fetchRequest error:nil];
    if (count > 0) {
        // DLog(@"Thread: %@ (isMain: %hhd), Validation failed!", [NSThread currentThread], [NSThread isMainThread]);
        if (outError != NULL) {
            NSString *errorString = NSLocalizedString(@"Object must have unique value for property", @"validation: nonunique property");
            NSDictionary *userInfoDict = @{ NSLocalizedDescriptionKey : errorString };
            *outError = [[NSError alloc] initWithDomain:nil code:0 userInfo:userInfoDict];
        }
        return NO;
    }
    // DLog(@"Thread: %@ (isMain: %hhd), Validation succeeded!", [NSThread currentThread], [NSThread isMainThread]);
    return YES;
}

The down side is the performance because it will be execute with every insertion. It will stop any userGUID duplicate before insertion in DB which also means the last version of the entity (although they were requested at the same time) will not be save although it may contains other updated fields.

More info here: http://goobbe.com/questions/4833509/core-data-saving-in-multiple-threads

[tonyarnold]

I think the best answer would be to have some kind of serial queue on a background thread to handle these requests. Performance would likely be better than the KVO-based workaround you've proposed, and you would not have to worry about multiple objects being created at the same time (so you could remove everything but an initial "does this object already exist" check).

I've take similar approaches to this using libraries like ReactiveCocoa, but you could easily use NSOperationQueue with a maxConcurrentOperationCount of 1, or a serial GCD queue. Just make sure that you route all requests through the one queue.

You could think about multiple "stages" of serial queues — setting gates where you de-duplicate, and then allowing creation, edits and deletes to be performed concurrently on many temporary contexts.

Lots of options :+1:

[dtrofimov]

The reason of duplication is creating a new local context in every -saveWithBlock:. Here is a part of its implementation:

NSManagedObjectContext *savingContext  = [NSManagedObjectContext MR_rootSavingContext];
NSManagedObjectContext *localContext = [NSManagedObjectContext MR_contextWithParent:savingContext];

A simple test reproducing the duplicates is as follows:

[MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
    [MyEntity MR_findFirstOrCreateByAttribute:@"id_" withValue:@"001" inContext:localContext];
    [NSThread sleepForTimeInterval:1];
}];
[MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
    [MyEntity MR_findFirstOrCreateByAttribute:@"id_" withValue:@"001" inContext:localContext];
    [NSThread sleepForTimeInterval:1];
}];

To overcome this effect, I had to create a method, that's almost equal to -saveWithBlock:, but uses the same localContext across all calls:

@implementation MagicalRecord (SerialSaving)

+ (void)serialSaveWithBlock:(void (^)(NSManagedObjectContext *))block completion:(MRSaveCompletionHandler)completion {
    static NSManagedObjectContext *localContext = nil;
    static dispatch_once_t onceToken;
    dispatch_once(&onceToken, ^{
        NSManagedObjectContext *savingContext  = [NSManagedObjectContext MR_rootSavingContext];
        localContext = [NSManagedObjectContext MR_contextWithParent:savingContext];
    });

    [localContext performBlock:^{
        [localContext MR_setWorkingName:NSStringFromSelector(_cmd)];

        if (block) {
            block(localContext);
        }

        [localContext MR_saveWithOptions:MRSaveParentContexts completion:completion];
    }];
}

@end

@tonyarnold, please, confirm if this method is OK. Also, I will appreciate if such functionality is finally added to MagicalRecord.

[LivioGama]

I have a question concerning the subject. Now that with Xcode 7 Apple added the unique constraint on some field, the pattern :

Publisher *user = [Publisher MR_findFirstByAttribute:@"identifier" withValue:userID inContext:localContext];
if (!user) {
    user = [Publisher MR_createEntityInContext:localContext];
}
user.identifier = userID;

shouldn't be necessary. Is it the case ? MagicalRecord support unique constraint or not ? Can I do this :

Publisher *user = [Publisher MR_createEntityInContext:localContext];
user.identifier = userID;

and have only one record of the user updated automatically (after saving) ?

[nickneiman]

@dtrofimov I'm thinking about implementing something similar. Have you run into any problems with your serialSaveWithBlock method?

[dtrofimov]

@nickneiman I used this serialSaveWithBlock in a sandbox project and it seems to be OK, including the test I wrote above. The only recommendation is to avoid doing something really long (> 1s) within serialSaveWithBlock, since it will block other operations until the long task is finished.

[victory1908]

hi how to call the

serialSaveWithBlock

? I have create a category but don't know what parameter to pass in?