drewmccormack / ensembles

A synchronization framework for Core Data.
MIT License
1.63k stars 132 forks source link

Unique constraints conflict resolution during import from iCloud #212

Closed pronebird closed 8 years ago

pronebird commented 8 years ago

Hi,

I've been playing with conflict resolution essentially with unique constraints. I've implemented ensemble delegate and resolution strategy but after returning YES in didFailToSaveMergedChangesInManagedObjectContext, context save still fails. Do you think you can give me some clue on what I am doing wrong with delegate methods?

- (void)persistentStoreEnsemble:(CDEPersistentStoreEnsemble *)ensemble didSaveMergeChangesWithNotification:(NSNotification *)notification {
    NSLog(@"Merge changes from ensemble.");

    [self.savingContext performBlockAndWait:^{
        [self.savingContext mergeChangesFromContextDidSaveNotification:notification];
    }];

    [self.context performBlockAndWait:^{
        [self.context mergeChangesFromContextDidSaveNotification:notification];
    }];
}

- (BOOL)persistentStoreEnsemble:(CDEPersistentStoreEnsemble *)ensemble shouldSaveMergedChangesInManagedObjectContext:(NSManagedObjectContext *)savingContext reparationManagedObjectContext:(NSManagedObjectContext *)reparationContext
{
    return YES;
}

- (BOOL)persistentStoreEnsemble:(CDEPersistentStoreEnsemble *)ensemble didFailToSaveMergedChangesInManagedObjectContext:(NSManagedObjectContext *)savingContext error:(NSError *)error reparationManagedObjectContext:(NSManagedObjectContext *)reparationContext
{
    BOOL shouldReattemptImport = NO;

    if([error.domain isEqualToString:NSCocoaErrorDomain])
    {
        if(error.code == NSManagedObjectConstraintMergeError)
        {
            NSArray *conflicts = error.userInfo[NSPersistentStoreSaveConflictsErrorKey];

            for(NSConstraintConflict *conflict in conflicts)
            {
                @autoreleasepool {
                    [self resolveConflict:conflict savingContext:savingContext reparationContext:reparationContext];
                }
            }

            shouldReattemptImport = YES;
        }
    }

    return shouldReattemptImport;
}

- (NSArray *)persistentStoreEnsemble:(CDEPersistentStoreEnsemble *)ensemble globalIdentifiersForManagedObjects:(NSArray *)objects {
    NSMutableArray *uniqueIdentifiers = [[NSMutableArray alloc] init];

    for(NSManagedObject *object in objects) {
        @autoreleasepool {
            NSAttributeDescription *attribute = [[object entity] MR_attributeDescriptionForName:@"uniqueIdentifier"];

            if(attribute) {
                id value = [object MR_valueForAttribute:attribute];

                if(value)
                {
                    [uniqueIdentifiers addObject:value];
                }
                else
                {
                    [uniqueIdentifiers addObject:[NSNull null]];
                }
            }
            else
            {
                [uniqueIdentifiers addObject:[NSNull null]];
            }
        }
    }

    return uniqueIdentifiers;
}

And this is my constraint resolution method. Very naive, resolves conflict by checking updatedAt property:

#pragma mark - Conflict resolution
#pragma mark -

- (void)resolveConflict:(NSConstraintConflict *)conflict
          savingContext:(NSManagedObjectContext *)savingContext
      reparationContext:(NSManagedObjectContext *)reparationContext
{
    NSManagedObject *databaseObject = conflict.databaseObject;
    NSMutableArray *conflictingObjects = [conflict.conflictingObjects mutableCopy];

    __block NSManagedObject *keeperObject = conflictingObjects.firstObject;
    NSAttributeDescription *updatedAtAttribute = [[keeperObject entity] MR_attributeDescriptionForName:@"updatedAt"];

    // to warn myself
    if(!updatedAtAttribute)
    {
        [NSException raise:NSGenericException format:@"Add updatedAt attribute on '%@' entity.", keeperObject.entity.name];

        return;
    }

    // blend in database object (never seen it being not nil so far)
    if(databaseObject && ![conflictingObjects containsObject:databaseObject])
    {
        [conflictingObjects addObject:databaseObject];
    }

    // pick up the latest version
    [savingContext performBlockAndWait:^{
        NSSortDescriptor *updatedAtSortDescription = [NSSortDescriptor sortDescriptorWithKey:@"updatedAt" ascending:NO];

        keeperObject = [[conflictingObjects sortedArrayUsingDescriptors:@[ updatedAtSortDescription ]] firstObject];
    }];

    NSParameterAssert(keeperObject);

    // remove the special object
    [conflictingObjects removeObject:keeperObject];

    [reparationContext performBlockAndWait:^{
        NSArray *allConflictingObjects = [conflictingObjects MR_entitiesInContext:reparationContext];

        // delete remaining objects in conflict
        for(NSManagedObject *object in allConflictingObjects) {
            [object MR_deleteEntityInContext:reparationContext];
        }
    }];
}
pronebird commented 8 years ago

I wasn't able to find anything regarding constraints conflict resolution. I also skimmed through your book and didn't find anything on that matter. (not written yet?)

I'd expect to be able to drop/merge objects but that didn't work and databaseObject was always nil in constraint error.

I am not sure how horrible that is but I set NSMergeByPropertyObjectTrumpMergePolicy on both savingContext and reparationContext during shouldSaveMergedChangesInManagedObjectContext and it seems to save without errors now.

- (BOOL)persistentStoreEnsemble:(CDEPersistentStoreEnsemble *)ensemble shouldSaveMergedChangesInManagedObjectContext:(NSManagedObjectContext *)savingContext reparationManagedObjectContext:(NSManagedObjectContext *)reparationContext
{
    savingContext.mergePolicy = NSMergeByPropertyObjectTrumpMergePolicy;
    reparationContext.mergePolicy = NSMergeByPropertyObjectTrumpMergePolicy;

    return YES;
}

- (BOOL)persistentStoreEnsemble:(CDEPersistentStoreEnsemble *)ensemble didFailToSaveMergedChangesInManagedObjectContext:(NSManagedObjectContext *)savingContext error:(NSError *)error reparationManagedObjectContext:(NSManagedObjectContext *)reparationContext
{
    NSLog(@"Failed to save merged changes: %@", error);

    return YES;
}
pronebird commented 8 years ago

Although constraints problem is gone, I am stuck with 100% CPU and never ending sync...

-[CDEICloudFileSystem repairEnsembleDirectory:completion:] line 176: Checking if repairs are needed in iCloud Drive
__77-[CDECloudManager retrieveRegistrationInfoForStoreWithIdentifier:completion:]_block_invoke_2 line 743: Downloading file at remote path: /CoreData-v5/stores/B3F16B0A-C8E5-4BDD-8C81-733DB76E0A03-1944-00000193009F6873
-[CDECloudManager createRemoteDirectories:withCompletion:] line 688: Creating remote directories
-[CDECloudManager importNewDataFilesWithCompletion:] line 159: Transferring new data files from cloud to event store
-[CDECloudManager importNewBaselineEventsWithCompletion:] line 143: Transferring new baselines from cloud to event store
-[CDEBaselineConsolidator consolidateBaselineWithCompletion:] line 65: Consolidating baselines
__61-[CDEBaselineConsolidator consolidateBaselineWithCompletion:]_block_invoke line 76: Found baselines with unique ids: (
    "874F9034-8FAB-450F-8CEB-C696216429A7-6459-00000CB9C534B5D9"
)
__61-[CDEBaselineConsolidator consolidateBaselineWithCompletion:]_block_invoke line 108: Baselines remaining that need merging: (
    "874F9034-8FAB-450F-8CEB-C696216429A7-6459-00000CB9C534B5D9"
)
__61-[CDEBaselineConsolidator consolidateBaselineWithCompletion:]_block_invoke_2 line 131: Finishing baseline consolidation
-[CDECloudManager importNewRemoteNonBaselineEventsWithCompletion:] line 127: Transferring new events from cloud to event store
-[CDEEventMigrator migrateEventsInFromFiles:completion:] line 141: Migrating file events to event store from paths: (
    "/var/mobile/Containers/Data/Application/5694DDB9-1E5B-4DEB-A0CD-3ABDCFBF8767/Library/Application Support/com.example.App/com.mentalfaculty.ensembles.eventdata/transitcache/CoreData-v5/download/24_68A072D0-A09C-41FC-A924-5AFE2ADD3925-6459-00000CB8E97B4BE4_15.cdeevent"
)
-[CDEEventIntegrator integrate:] line 311: Integrating new events into main context
__32-[CDEEventIntegrator integrate:]_block_invoke line 329: Baseline has changed. Will carry out full integration of the persistent store.
-[CDEEventIntegrator repairWithMergeEventBuilder:error:] line 811: Repairing context after integrating changes
-[CDEEventIntegrator commitWithMergeEventBuilder:error:] line 1048: Committing merge changes to store
-[CDESaveMonitor storePreSaveChangesFromUpdatedObjects:] line 127: Storing pre-save changes from updated objects
screenshot
pronebird commented 8 years ago

Dropped the app and reinstalled. Initial import took about a minute or two with 100% CPU utilization. iPhone 5 is pretty hot now wow :)

I am in debug mode with verbose logging and have a de-dupping of seed data happening using uniqueIdentifiers that I provide to Ensembles. (I have about 1400 records in Seed database). Two devices and two seed dbs, so about 2800 objects to check.

Thank you for that marvelous library! It seems to work well so far, even after all the struggling prepping everything from seed data to models to work with it!

pronebird commented 8 years ago

To be fair, after syncing between both devices for a little bit my iPhone 5 is back to being stuck at:

-[CDESaveMonitor storePreSaveChangesFromUpdatedObjects:] line 127: Storing pre-save changes from updated objects

and 99% CPU usage.

pronebird commented 8 years ago

I think using willSave in one of my subclasses was a big mistake to update updatedAt property.

drewmccormack commented 8 years ago

Yes, that could be it. Each time you change something in willSave, Core Data performs all validation etc again. If you keep changing things, it will get in an infinite loop. At the very least, make sure you don’t update if you just did it. Eg. Check if the date stored is less than 5 secs old, and if so, don’t set it again. Something like that.

Drew

On 31 Dec 2015, at 02:12, Andrey Mikhaylov notifications@github.com wrote:

I think using willSave in one of my subclasses was a big mistake to update updatedAt property.

— Reply to this email directly or view it on GitHub https://github.com/drewmccormack/ensembles/issues/212#issuecomment-168102705.

pronebird commented 8 years ago

@drewmccormack thanks Drew! Yes that was it. I've changed my code to check if property was already changed and if anything else was changed, otherwise it would skip an update.

- (void)willSave {
    if(![self isDeleted])
    {
        id changedUpdatedAt = self.changedValues[MyModelAttributes.updatedAt];

        if( (!changedUpdatedAt || self.updatedAt == nil) && self.changedValues.count > 0 )
        {
            self.updatedAt = [NSDate date];
        }
    }

    [super willSave];
}