Assertion for nil MOC, take 2 #234

Closed
wants to merge 2 commits into
from

3 participants

@notjosh

Okay, this should satisfy standard use cases of:

  1. Non-faulted MO with context
  2. Faulted MO without context (will throw assertion)
  3. Non-faulted MO without context (will work a-okay)

And, to be sure, I added a test to cover the use case for non-faulted/missing MOC.

@jspahrsummers jspahrsummers self-assigned this Feb 10, 2014
@jspahrsummers jspahrsummers commented on the diff Feb 10, 2014
Mantle/MTLManagedObjectAdapter.m
@@ -132,6 +132,7 @@ - (id)modelFromManagedObject:(NSManagedObject *)managedObject processedObjects:(
NSAssert(entity != nil, @"%@ returned a nil +entity", managedObject);
NSManagedObjectContext *context = managedObject.managedObjectContext;
+ NSAssert(!(context == nil && managedObject.isFault), @"%@ returned a nil +managedObjectContext for faulted managed object", managedObject);
@jspahrsummers
Mantle member
jspahrsummers added a line comment Feb 10, 2014

How about putting this into an if for readability:

if (managedObject.isFault) {
    NSAssert(context != nil, @"");
}

It'll still be compiled out when assertions are disabled, but it makes the condition more understandable (to me).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jspahrsummers jspahrsummers commented on the diff Feb 10, 2014
MantleTests/MTLManagedObjectAdapterSpec.m
@@ -314,6 +314,49 @@
});
});
+describe(@"without context", ^{
+ __block NSManagedObjectContext *context; // used for entity descriptions, not for persistence
@jspahrsummers
Mantle member
jspahrsummers added a line comment Feb 10, 2014

Can't this be obtained from the NSManagedObjectModel instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jspahrsummers jspahrsummers commented on the diff Feb 10, 2014
MantleTests/MTLManagedObjectAdapterSpec.m
+
+ beforeEach(^{
+ context = [[NSManagedObjectContext alloc] initWithConcurrencyType:NSMainQueueConcurrencyType];
+ expect(context).notTo.beNil();
+
+ context.undoManager = nil;
+ context.persistentStoreCoordinator = persistentStoreCoordinator;
+
+ date = [NSDate date];
+ numberString = @"123456789";
+ string = @"foobar";
+
+ NSEntityDescription *parentEntity = [NSEntityDescription entityForName:@"Parent" inManagedObjectContext:context];
+ expect(parentEntity).notTo.beNil();
+
+ parent = (MTLParent *)[[NSManagedObject alloc] initWithEntity:parentEntity insertIntoManagedObjectContext:nil];
@jspahrsummers
Mantle member
jspahrsummers added a line comment Feb 10, 2014

Personally, I prefer to cast to id in cases like this, since it makes the cast less fragile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jspahrsummers
Mantle member

Looks pretty good, thank you! I just have some minor stylistic notes. 🌠

@robb robb added the Core Data label Mar 6, 2014
@jspahrsummers
Mantle member

Core Data support has now moved to a separate repository. Please reopen this PR against that repo if you'd still like to see the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment