Validation errors handling

[andreacremaschi]

hi again,

I noticed that when the background context can't be saved at the end of the marshaling process, i.e. for a validation issue, the request succeed and it returns an array of objectIDs anyway. Is it correct? The new data wasn't saved in the main context and the API just returns an array of the old values of the local persisted data.

Here is the code:

+ (NSArray *)objectIDsForRecords:(NSArray *)records
                   onMainContext:(NSManagedObjectContext *)mainContext
           fromBackgroundContext:(NSManagedObjectContext *)backgroundContext {
    [mainContext MMRecord_startObservingWithContext:backgroundContext];

    NSError *coreDataError = nil;
    if ([backgroundContext save:&coreDataError] == NO) {
        NSString *coreDataErrorString = [NSString stringWithFormat:@"Core Data error occurred with code: %d, description: %@",
                                                                   coreDataError.code,
                                                                   coreDataError.localizedDescription];
        NSString *errorDescription = [NSString stringWithFormat:@"Unable to save background context while populating records. MMRecord import operation unsuccessful. %@",
                                                                coreDataErrorString];

        MMRecordOptions *options = [self currentOptions];
        MMRecordDebugger *debugger = options.debugger;
        NSDictionary *parameters = [debugger parametersWithKeys:@[MMRecordDebuggerParameterErrorDescription] values:@[errorDescription]];

        [debugger handleErrorCode:MMRecordErrorCodeCoreDataSaveError withParameters:parameters];
    }

    [mainContext MMRecord_stopObservingWithContext:backgroundContext];

    NSMutableArray *objectIDs = [NSMutableArray array];

    for (MMRecord *record in records) {
        [objectIDs addObject:[record objectID]];
    }

    return objectIDs;
}

[cnstoll]

Doh. Nice find.

What would you expect to happen in this case? Out of curiosity, what is the error that is preventing the save from taking place? Is it recoverable? Do you think that there needs to be some mechanism to try to prevent the failure, or should it just terminate here and cause the failureBlock to fire with the Core Data error code?

Thanks,

• Conrad

[andreacremaschi]

Well, I think that there are two different issues to cover here: when there are Coredata saving issues I would expect the failure block to be invoked with the core data error object returned as a parameter.

Validation errors should be treated differently: the saving that occurs at this point is not intended to persist data but to propagate the new records in the user's managed object context. I would expect validation errors to occur instead when the user try to persist data (i.e. calling [context save:] on his managedObjectContext). This is useful when data contained in the remote server is "dirty" and does not conform to the rules defined in the local model.

I submit a PR that extends NSManagedObjectContext to make it skip validation: https://github.com/mutualmobile/MMRecord/pull/97 I used associated objects as it was faster to implement, but a subclass could be used instead. What do you think?

[cnstoll]

Isn't this a case where having MMRecord be a subclass of NSManagedObject is actually a good thing? You could just implement that method on your own record classes and control the logic however you like. That seems to me like the better way to handle this versus baking it into the library completely.

The place where I'd love to see MMRecord improve is by really and truly implementing and playing nicely with this sort of validation as a first class citizen. I've thought about that before but never really come up with a good way to do it that makes sense. Part of it is because the Core Data validation seems a bit arbitrary to me and hasn't really ever even felt like something I wanted to use...so I was hesitant to base the MMRecord implementation solely off of that. I'm open to suggestions for how that might work though.

[andreacremaschi]

As a user I can see two possible uses for validation: in the simple one, I want to let in my local data only records that conform to my validation rules and have an error returned in all the other cases. One possible way to support this scenario could be just failing on error while saving in + (NSArray *)objectIDsForRecords:(NSArray *)records onMainContext:(NSManagedObjectContext *)mainContext fromBackgroundContext:(NSManagedObjectContext *)backgroundContext. The user could even modify the JSON in the recordPrePopulationBlock, check for sanity and prevent recurring validation errors.. This just works, but in my opinion it is not the best way to support validation.

The second scenario is the one described above: I have some "dirty" data on a remote server and I want the app's user to have the chance to correct data before committing it to the local persisting store. In this scenario, this method: + (NSArray *)objectIDsForRecords:(NSArray *)records onMainContext:(NSManagedObjectContext *)mainContext fromBackgroundContext:(NSManagedObjectContext *)backgroundContext should not fail on validation, but propagate all the imported data to the parent context. The validation checks now will be triggered when the parent context tries to save. This would a better way in my opinion, since the user can handle validation errors as he's used to, following Core Data's guidelines.