diff --git a/Code/CoreData/RKManagedObjectMappingOperationDataSource.m b/Code/CoreData/RKManagedObjectMappingOperationDataSource.m index 4efb8837..6d9dabe2 100644 --- a/Code/CoreData/RKManagedObjectMappingOperationDataSource.m +++ b/Code/CoreData/RKManagedObjectMappingOperationDataSource.m @@ -123,6 +123,20 @@ static id RKMutableCollectionValueWithObjectForKeyPath(id object, NSString *keyP return nil; } +static BOOL RKDeleteInvalidNewManagedObject(NSManagedObject *managedObject) +{ + if ([managedObject isKindOfClass:[NSManagedObject class]] && [managedObject managedObjectContext] && [managedObject isNew]) { + NSError *validationError = nil; + if (! [managedObject validateForInsert:&validationError]) { + RKLogDebug(@"Unsaved NSManagedObject failed `validateForInsert:` - Deleting object from context: %@", validationError); + [managedObject.managedObjectContext deleteObject:managedObject]; + return YES; + } + } + + return NO; +} + @interface RKManagedObjectDeletionOperation : NSOperation - (id)initWithManagedObjectContext:(NSManagedObjectContext *)managedObjectContext; @@ -296,17 +310,7 @@ extern NSString * const RKObjectMappingNestingAttributeKeyName; - (BOOL)commitChangesForMappingOperation:(RKMappingOperation *)mappingOperation error:(NSError **)error { if ([mappingOperation.objectMapping isKindOfClass:[RKEntityMapping class]]) { - [self emitDeadlockWarningIfNecessary]; - - // Validate unsaved objects - if ([mappingOperation.destinationObject isKindOfClass:[NSManagedObject class]] && [(NSManagedObject *)mappingOperation.destinationObject isNew]) { - NSError *validationError = nil; - if (! [(NSManagedObject *)mappingOperation.destinationObject validateForInsert:&validationError]) { - RKLogDebug(@"Unsaved NSManagedObject failed `validateForInsert:` - Deleting object from context: %@", validationError); - [self.managedObjectContext deleteObject:mappingOperation.destinationObject]; - return YES; - } - } + [self emitDeadlockWarningIfNecessary]; NSArray *connections = [(RKEntityMapping *)mappingOperation.objectMapping connections]; if ([connections count] > 0 && self.managedObjectCache == nil) { @@ -315,7 +319,16 @@ extern NSString * const RKObjectMappingNestingAttributeKeyName; if (error) *error = localError; return NO; } - + + // Delete the object immediately if there are no connections that may make it valid + if ([connections count] == 0 && RKDeleteInvalidNewManagedObject(mappingOperation.destinationObject)) return YES; + + // Attempt to establish the connections and delete the object if its invalid once we are done + NSOperationQueue *operationQueue = self.operationQueue ?: [NSOperationQueue currentQueue]; + NSBlockOperation *deletionOperation = [NSBlockOperation blockOperationWithBlock:^{ + RKDeleteInvalidNewManagedObject(mappingOperation.destinationObject); + }]; + for (RKConnectionDescription *connection in connections) { RKRelationshipConnectionOperation *operation = [[RKRelationshipConnectionOperation alloc] initWithManagedObject:mappingOperation.destinationObject connection:connection managedObjectCache:self.managedObjectCache]; [operation setConnectionBlock:^(RKRelationshipConnectionOperation *operation, id connectedValue) { @@ -330,10 +343,13 @@ extern NSString * const RKObjectMappingNestingAttributeKeyName; } }]; if (self.parentOperation) [operation addDependency:self.parentOperation]; - NSOperationQueue *operationQueue = self.operationQueue ?: [NSOperationQueue currentQueue]; + [deletionOperation addDependency:operation]; [operationQueue addOperation:operation]; RKLogTrace(@"Enqueued %@ dependent upon parent operation %@ to operation queue %@", operation, self.parentOperation, operationQueue); } + + // Enqueue our deletion operation for execution after all the connections + [operationQueue addOperation:deletionOperation]; // Handle tombstone deletion by predicate if ([(RKEntityMapping *)mappingOperation.objectMapping deletionPredicate]) { diff --git a/Tests/Logic/CoreData/RKManagedObjectMappingOperationDataSourceTest.m b/Tests/Logic/CoreData/RKManagedObjectMappingOperationDataSourceTest.m index 5c88458c..2550ad5d 100644 --- a/Tests/Logic/CoreData/RKManagedObjectMappingOperationDataSourceTest.m +++ b/Tests/Logic/CoreData/RKManagedObjectMappingOperationDataSourceTest.m @@ -1286,4 +1286,36 @@ expect(catsWithID419).to.haveCountOf(1); } +- (void)testManagedObjectsMappedWithRequiredRelationshipsThatAreSetByConnectionsAreNotPrematurelyDeleted +{ + RKManagedObjectStore *managedObjectStore = [RKTestFactory managedObjectStore]; + RKFetchRequestManagedObjectCache *managedObjectCache = [RKFetchRequestManagedObjectCache new]; + RKManagedObjectMappingOperationDataSource *mappingOperationDataSource = [[RKManagedObjectMappingOperationDataSource alloc] initWithManagedObjectContext:managedObjectStore.persistentStoreManagedObjectContext + cache:managedObjectCache]; + mappingOperationDataSource.operationQueue = [NSOperationQueue new]; + + NSManagedObject *cat = [NSEntityDescription insertNewObjectForEntityForName:@"Cat" inManagedObjectContext:managedObjectStore.persistentStoreManagedObjectContext]; + [cat setValue:@(12345) forKey:@"railsID"]; + + NSDictionary *representation = @{ @"name": @"Blake Watters", @"requiredCatID": @(12345) }; + RKEntityMapping *catMapping = [RKEntityMapping mappingForEntityForName:@"Cat" inManagedObjectStore:managedObjectStore]; + catMapping.identificationAttributes = @[ @"railsID" ]; + + RKEntityMapping *strictHumanMapping = [RKEntityMapping mappingForEntityForName:@"StrictHuman" inManagedObjectStore:managedObjectStore]; + [strictHumanMapping addAttributeMappingsFromDictionary:@{ @"name": @"name" }]; + [strictHumanMapping addPropertyMapping:[RKAttributeMapping attributeMappingFromKeyPath:@"requiredCatID" toKeyPath:@"favoriteCatID"]]; + [strictHumanMapping addConnectionForRelationship:@"requiredCat" connectedBy:@{ @"favoriteCatID": @"railsID" }]; + + RKMapperOperation *mapper = [[RKMapperOperation alloc] initWithRepresentation:representation mappingsDictionary:@{ [NSNull null]: strictHumanMapping }]; + mapper.mappingOperationDataSource = mappingOperationDataSource; + [mapper start]; + [mappingOperationDataSource.operationQueue waitUntilAllOperationsAreFinished]; + + RKHuman *blake = [mapper.mappingResult firstObject]; + expect(blake.name).to.equal(@"Blake Watters"); + expect(blake.managedObjectContext).notTo.beNil(); + expect([blake isDeleted]).to.beFalsy(); + expect([blake valueForKey:@"requiredCat"]).to.equal(cat); +} + @end diff --git a/Tests/Models/Data Model.xcdatamodel/elements b/Tests/Models/Data Model.xcdatamodel/elements index 640aa70f..12588e6e 100644 Binary files a/Tests/Models/Data Model.xcdatamodel/elements and b/Tests/Models/Data Model.xcdatamodel/elements differ diff --git a/Tests/Models/Data Model.xcdatamodel/layout b/Tests/Models/Data Model.xcdatamodel/layout index 84d42c9d..f4422cc0 100644 Binary files a/Tests/Models/Data Model.xcdatamodel/layout and b/Tests/Models/Data Model.xcdatamodel/layout differ