Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly update a model after creation #984

Closed
mathewdgardner opened this issue Mar 13, 2015 · 13 comments
Closed

Properly update a model after creation #984

mathewdgardner opened this issue Mar 13, 2015 · 13 comments

Comments

@mathewdgardner
Copy link

I have a model called Order that extends NSManagedObject. In it are methods to save locally and remotely to an api. I want to be able to save the order locally and POST to the api independent of each other. However, when the POST is successful I want to update the order I just made locally.

I create an Order in memory:

  Order* o = [Order MR_createEntity];

I save the Order locally:

    [MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
        NSLog(@"Saving order locally");

        Order* localOrder = [Order MR_createInContext:localContext];
        localOrder.type = self.type;
        localOrder.transactionType = self.transactionType;
        localOrder.total = self.total;
    } completion:^(BOOL success, NSError *error) {
        if(success)  {
            NSLog(@"Order local save succeeded");
            [delegate createLocalSucceeded:self];
        } else  {
            NSLog(@"Order local save failed\n%@", error);
            [delegate createLocalFailed:error];
        }
    }];

I POST to the api using AFNetworking and in the POST success block I have:

  orderId = [orderJson valueForKey:@"id"];
  confirmationCode = [NSString stringWithFormat:@"%@", [orderJson valueForKey:@"confirmationCode"]];
  status = [orderJson valueForKey:@"status"];
  createdAt = [orderJson valueForKey:@"createdAt"];`

  [self.managedObjectContext MR_saveToPersistentStoreWithCompletion:^(BOOL success, NSError *error) {
    if(success) {
      NSLog(@"Order update success");

      [delegate createRemoteSucceeded:self];
    } else {
      NSLog(@"Order update failed");

      [delegate createRemoteFailed:error];
   }];

With the code as it is it creates an Order locally as desired with some database fields missing (will come from the api). Then after the api comes back with more data I try to save the updated fields but instead of updating, a new record is created with all fields populated.

So my question is, how do I update my Order without duplicating it in the local database? I suspect this has to do with contexts and using them correctly.

Here is an example of my sqlite table after one Order creation with update attempt:

Z_PK Z_ENT Z_OPT ZCONFIRMATIONCODE ZCREATEDAT ZORDERID ZSTATUS ZTOTAL ZTRANSACTIONTYPE
90 1 1 NULL NULL NULL NULL 20.00 cash
91 1 1 93 2015-03-13T22:14:15.532Z 9fd6db72-4c3f-4b57-ac1c-c89e4d1e522e successful 20.00 cash
@Alydyn
Copy link

Alydyn commented Mar 14, 2015

The way you are using [MagicalRecord saveWithBlock:] and your self object will almost certainly cause a Core Data concurrency violation. You should really turn on concurrency debugging. Why do you need to use the +saveWithBlock: method in the first place? Are you able to assign properties to the new object you created (with Order *o = [Order MR_createEntity]) and then save that directly?

@mathewdgardner
Copy link
Author

I am able to save to the object I created with Order *o = [Order MR_createEntity]. I use saveWithBlock: because I need everything to be asynchronous. I have success/fail callback methods from a protocol I have written, OrderDelegate.

What I'm seeing is that after a record is created, the object I have in memory doesn't update the record when it changes. If I try to save it manually after changes have been made it creates a whole new record instead of updating it.

I tried the synchronous way but I get another problem. After the new record is made, the object I have in memory has all of its fields turned to nil as if the object has been flushed.

The workflow I need is as follows:

  1. Collect data and store it in memory, the Order object. Order does indeed extend NSManagedObject.
  2. Save the new record to the local datastore.
  3. Send this Order to the API.
  4. Update the local Order from data that comes back from the API.

This all happens very quickly. In the time between saving the record locally and the API coming back with new data I still hold the Order in memory. What is proper way to update the record that already exists in the datastore?

Please forgive my ignorance in this regarding Core Data. I've been doing a lot of reading and I have yet found the answer. Please advise!

@mathewdgardner
Copy link
Author

I took it a step further and generate a UUID on Order on creation. Then, when I go to update the Order with new fields I instead findByAttribute using the UUID. It successfully finds the record. Changing its properties does nothing. Changing the properties then saving creates a duplicate record.

This seems so strange to me. It seems like I will have to use this method but delete the found record when updating it since a new one will be replacing it.

@thomassnielsen
Copy link
Contributor

When updating an existing record from the API you would need to find the existing record and either update it or delete it and create a new one.

I often use this pattern:

[MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
    // Assuming dict is a dictionary of parsed data from the API
    Order *order = [Order findFirstByAttribute:@"uuid" withValue:dict[@"uuid"] inContext:localContext];
    if (!order)
        order = [Order MR_createEntityInContext:localContext];

    // Set data on order object
}];

If you have an object already in memory and want to save changes in a block:

// Somewhere above you got this
// Order *myOrder = […]
[MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
  Order *orderInContext = [myOrder MR_inContext:localContext];
  orderInContext.someValue = @"newValue";
} completion:^(BOOL success, NSError *error){
  // Do any work you need to do post-saving here
}];

Did you turn on concurrency debugging? Any concurrency issues will make you go mad by giving you all sorts of weird problems. There is more info about it here: https://github.com/magicalpanda/MagicalRecord/blob/develop/Docs/Threads.md

@Alydyn
Copy link

Alydyn commented Mar 17, 2015

1.) Have you turned on concurrency debugging yet?
2.) MR_createEntity does not save the object, only create it. Also you really really should use the version that takes a context, especially if you are multi-threading with Core Data
3.) If you create the object with MR_createEntity you must save it's context before you can access that object in any other context. And conversely, if you make changes to an object in a second context you must save that one, and possibly merge in the changes in the first context, before those changes will be reflected.
4.) To easily access a previously saved object, inside your saveWithBlock: you should use:

Order *o = [order MR_inContext:localContext];

@Alydyn
Copy link

Alydyn commented Mar 17, 2015

@thomassnielsen just beat me to it!
Seriously though, I've mentioned it before and now so has @thomassnielsen ... turn on concurrency debugging!!

@mathewdgardner
Copy link
Author

When I enable concurrency debugging my app stops on startup breaking on NSManagedObjectContext+MagicalRecord.m:243

NSString *workingName = [[self userInfo] objectForKey:kMagicalRecordNSManagedObjectContextWorkingName];

The break states

Thread 1: EXC_BAD_INSTRUCTION(code=EXC_I386_INVOP,subcode=0x0)
- (NSString *) MR_workingName;
{
    NSString *workingName = [[self userInfo] objectForKey:kMagicalRecordNSManagedObjectContextWorkingName];
    if (nil == workingName)
    {
        workingName = @"UNNAMED";
    }
    return workingName;
}

I assume this is from AppDelegate.m where in

- (BOOL)application:(UIApplication*)application didFinishLaunchingWithOptions:(NSDictionary*)launchOptions

I have

[MagicalRecord setupCoreDataStackWithAutoMigratingSqliteStoreNamed:@"Orders.sqlite"];

Beyond that, without the concurrency debugging turned on, when I use the method above mentioned I have Order* order = [Order MR_createEntity]; in a view controller. I know that doesn't save the record. Later in the the view controller it calls [order createLocalOrder] on the order itself to save locally. There I have

    [MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
        NSLog(@"Saving order locally");

        Order* localOrder = [Order MR_createInContext:localContext];
        localOrder.transactionType = self.transactionType;
        localOrder.total = self.total;
    } completion:^(BOOL success, NSError *error) {
        if(success)
        {
            NSLog(@"Order local save succeeded");
            [self createRemoteOrder];
            [delegate createLocalSucceeded:self];
        }
        else
        {
            NSLog(@"Order local save failed\n%@", error);
            [delegate createLocalFailed:error];
        }
    }];

This succeeds and only after this thread is complete does it start the next step. Then in the API POST's success block it calls - (void)updateLocal:(NSDictionary*)dictionary which does:

    [MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
        NSLog(@"Saving order locally");

        Order* localOrder = [self MR_inContext:localContext];
        localOrder.orderId = [dictionary valueForKey:@"id"];
        localOrder.confirmationCode = [NSString stringWithFormat:@"%@", [dictionary valueForKey:@"confirmationCode"]];
        localOrder.status = [dictionary valueForKey:@"status"];
        localOrder.createdAt = [dictionary valueForKey:@"createdAt"];
    } completion:^(BOOL success, NSError *error) {
        if(success)
        {
            NSLog(@"Order local save succeeded");
            [delegate updateLocalSucceeded:self];
        }
        else
        {
            NSLog(@"Order local save failed\n%@", error);
            [delegate updateLocalFailed:error];
        }
    }];

Doing it this way results in the log output

-[NSManagedObjectContext(MagicalSaves) MR_saveWithOptions:completion:](0x7fa8a973d730) NO CHANGES IN ** UNNAMED ** CONTEXT - NOT SAVING

Stepping through starting on Order* localOrder = [self MR_inContext:localContext]; I see that localOrder is nil.

Since these all occur in each success block I don't see it being a concurrency issue. Please forgive me if I'm being thick as I am new to core data.

@Alydyn
Copy link

Alydyn commented Mar 17, 2015

What version of MagicalRecord are you using? You should try version 2.3beta5. That should resolve the concurrency exception on the working name. Then you can turn concurrency debugging back on.

Next, you say that you call [order createLocalOrder] on the order itself to save locally. If what you posted following that is what is in that method, it is important to note that you still do not actually save that original order, the one you created in a view controller with Order *order = [Order MR_createEntity].
You need to actually call [self.managedObjectContext MR_saveToPersistentStoreAndWait] or similar in that method (BEFORE the saveWithBlock: call) to save that context (unless you are doing so before the code you posted). And again, in that first saveWithBlock: call you are going to get a concurrency crash because you are accessing self, an NSManagedObject, on a queue other than it's own (e.g. when you call localOrder.transactionType = self.transactionType). Just because it succeeds doesn't mean that it's not also a concurrency violation.
Most likely you are getting nil for Order *localOrder = [self MR_inContext:localContext], because self is still a temporary object (you can tell if this is the case because it's objectID will be prefixed with "t"). Fetching a local context version will not work for a temporary object because that object is not actually in the persistent store yet, so there is nothing for it to fetch. It isn't in the store yet because, again, I don't think you are actually saving that initial, first object.
If you are indeed using the master branch (version 2.2), then this is what you have for the MR_inContext: method:

- (id) MR_inContext:(NSManagedObjectContext *)otherContext
{
    NSError *error = nil;
    NSManagedObject *inContext = [otherContext existingObjectWithID:[self objectID] error:&error];
    [MagicalRecord handleErrors:error];

    return inContext;
}

Versus what is now in v2.3beta5:

- (id) MR_inContext:(NSManagedObjectContext *)otherContext
{
    NSError *error = nil;

    if ([[self objectID] isTemporaryID])
    {
        BOOL success = [[self managedObjectContext] obtainPermanentIDsForObjects:@[self] error:&error];
        if (!success)
        {
            [MagicalRecord handleErrors:error];
            return nil;
        }
    }

    error = nil;

    NSManagedObject *inContext = [otherContext existingObjectWithID:[self objectID] error:&error];
    [MagicalRecord handleErrors:error];

    return inContext;
}

You'll notice in v2.3beta5 it attempts to resolve temporary ID problems for you. I suspect if you put a break on line 225 in NSManagedObject+MagicalRecord.m and p (BOOL)[[self objectID] isTemporaryID], you'll see it is.

@mathewdgardner
Copy link
Author

Changing from

- (void)createLocal
{
    [MagicalRecord saveWithBlock:^(NSManagedObjectContext *localContext) {
        DDLogInfo(@"Saving order locally");

        Order* localOrder = [Order MR_createInContext:localContext];
        localOrder.transactionType = self.transactionType;
        localOrder.total = self.total;
    } completion:^(BOOL success, NSError *error) {
        if(success)
        {
            DDLogInfo(@"Order local save succeeded");
            [self createRemote];
            [delegate createLocalSucceeded:self];
        }
        else
        {
            DDLogInfo(@"Order local save failed\n%@", error);
            [delegate createLocalFailed:error];
        }
    }];
}

to

- (void)createLocal
{
    [self.managedObjectContext MR_saveToPersistentStoreWithCompletion:^(BOOL success, NSError *error) {
        if(success)
        {
            DDLogInfo(@"Order local save succeeded");
            [delegate createLocalSucceeded:self];
        }
        else
        {
            DDLogInfo(@"Order local save failed\n%@", error);
            [delegate createLocalFailed:error];
        }
    }];
}

also saves the record but I notice that all the properties in the order, self, are now nil after the save completes. Is saving a model supposed to flush its properties? Is there a way to preserve them?

@thomassnielsen
Copy link
Contributor

Are you sure that the order does have its properties set when you call save? I've never seen Core Data saving flush properties an object.

Can you try this and post the results?

NSLog(@"Before: %@", self.someProperty);
[self.managedObjectContext MR_saveToPersistentStoreAndWait];
NSLog(@"After: %@", self.someProperty);

@mathewdgardner
Copy link
Author

- (void)createLocal
{
    /*[self.managedObjectContext MR_saveToPersistentStoreWithCompletion:^(BOOL success, NSError *error) {
        if(success)
        {
            DDLogInfo(@"Order local save succeeded");
            [self createRemote];
            [delegate createLocalSucceeded:self];
        }
        else
        {
            DDLogInfo(@"Order local save failed\n%@", error);
            [delegate createLocalFailed:error];
        }
    }];*/

    NSLog(@"Before: %@", self.transactionType);
    [self.managedObjectContext MR_saveToPersistentStoreAndWait];
    NSLog(@"After: %@", self.transactionType);
}
2015-03-18 10:34:21.279 test[29645:6581429] Before: cash
2015-03-18 10:34:22.862 test[29645:6581429] -[NSManagedObjectContext(MagicalSaves) MR_saveWithOptions:completion:](0x7f835049f030) → Saving <NSManagedObjectContext (0x7f835049f030): *** DEFAULT ***> on *** MAIN THREAD ***
2015-03-18 10:34:22.863 test[29645:6581429] -[NSManagedObjectContext(MagicalSaves) MR_saveWithOptions:completion:](0x7f835049f030) → Save Parents? 1
2015-03-18 10:34:22.863 test[29645:6581429] -[NSManagedObjectContext(MagicalSaves) MR_saveWithOptions:completion:](0x7f835049f030) → Save Synchronously? 1
2015-03-18 10:34:22.863 test[29645:6581429] -[NSManagedObjectContext(MagicalRecord) MR_contextWillSave:](0x7f835049f030) Context DEFAULT is about to save. Obtaining permanent IDs for new 1 inserted objects
2015-03-18 10:34:22.864 test[29645:6581429] -[NSManagedObjectContext(MagicalSaves) MR_saveWithOptions:completion:](0x7f835042c240) → Saving <NSManagedObjectContext (0x7f835042c240): *** BACKGROUND SAVING (ROOT) ***> on *** MAIN THREAD ***
2015-03-18 10:34:22.865 test[29645:6581429] -[NSManagedObjectContext(MagicalSaves) MR_saveWithOptions:completion:](0x7f835042c240) → Save Parents? 1
2015-03-18 10:34:22.865 test[29645:6581429] -[NSManagedObjectContext(MagicalSaves) MR_saveWithOptions:completion:](0x7f835042c240) → Save Synchronously? 1
2015-03-18 10:34:22.884 test[29645:6581429] -[NSManagedObjectContext(MagicalRecord) MR_contextWillSave:](0x7f835042c240) Context BACKGROUND SAVING (ROOT) is about to save. Obtaining permanent IDs for new 1 inserted objects
2015-03-18 10:34:22.885 test[29645:6581429] __70-[NSManagedObjectContext(MagicalSaves) MR_saveWithOptions:completion:]_block_invoke21(0x7f835042c240) → Finished saving: <NSManagedObjectContext (0x7f835042c240): *** BACKGROUND SAVING (ROOT) ***> on *** MAIN THREAD ***
2015-03-18 10:34:32.700 test[29645:6581429] After: (null)

I should mention I'm using version 2.2 pod 'MagicalRecord', '~> 2.2'

@Alydyn
Copy link

Alydyn commented Mar 18, 2015

You should use v2.3beta5, as I previously mentioned.

@mathewdgardner
Copy link
Author

So I was doing other things with MagicalRecord / Core Data with other models and having to mimic the API's backend database structure to cache its responses and I thought it was going to get really hairy real quick, but it didn't. Everything worked pretty much on the first attempt. I noticed one major difference between my Order model and the other ones I just created. Order was my first model and my introduction to this library and Core Data. I had @synthesized the Order entity's attributes but allowed the other models to remain the generated @dynamic. Using the previously mentioned

- (void)createLocal
{
    [self.managedObjectContext MR_saveToPersistentStoreWithCompletion:^(BOOL success, NSError *error) {
        if(success)
        {
            DDLogInfo(@"Order local save succeeded");
            [self createRemote];
            [delegate createLocalSucceeded:self];
        }
        else
        {
            DDLogInfo(@"Order local save failed\n%@", error);
            [delegate createLocalFailed:error];
        }
    }];
}



- (void)updateLocal
{    
    [self.managedObjectContext MR_saveToPersistentStoreWithCompletion:^(BOOL success, NSError *error) {
        if(success)
        {
            DDLogInfo(@"Order local save succeeded");
            [delegate updateLocalSucceeded:self];
        }
        else
        {
            DDLogInfo(@"Order local save failed\n%@", error);
            [delegate updateLocalFailed:error];
        }
    }];
}

and @dynamic solved the problem for me. Thank you guys for your stellar support and patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants