Skip to content
This repository has been archived by the owner on Dec 5, 2019. It is now read-only.

mergeValuesOfModel:forKeysFromManagedObject doesn't handle default key mappings #2

Open
jhosteny opened this issue Sep 13, 2014 · 2 comments
Labels

Comments

@jhosteny
Copy link

If we implement the selector -mergeValueForKey:fromManagedObject:, and not -mergeValuesForKeysFromManagedObject:, the enumeration occurs over the keys provided by
+managedObjectKeysByPropertyKey.

- (void)mergeValuesOfModel:(id<MTLManagedObjectSerializing>)model forKeysFromManagedObject:(NSManagedObject *)managedObject {
    if ([model respondsToSelector:@selector(mergeValuesForKeysFromManagedObject:)]) {
        [model mergeValuesForKeysFromManagedObject:managedObject];
    } else if ([model respondsToSelector:@selector(mergeValueForKey:fromManagedObject:)]) {
        [[model.class managedObjectKeysByPropertyKey] enumerateKeysAndObjectsUsingBlock:^(NSString *key, NSString *managedObjectKey, BOOL *stop) {
            [self mergeValueOfModel:model forKey:key fromManagedObject:managedObject];
        }];
    }
}

It seems like this is not the expected behavior. Shouldn't it call -managedObjectKeyForKey: to ensure that it includes default mappings? Another possibility is to just construct the _managedObjectKeysByPropertyKey by utilizing this method in the initializer.

If this does seem incorrect, I'm happy to work on a patch for the preferred fix.

@jspahrsummers
Copy link
Member

Sorry, I'm having trouble understanding the behavior you're seeing vs. the behavior you expect.

Based on that code, it looks like it enumerates over those property keys that have managed object mappings. That sounds correct to me?

@jhosteny
Copy link
Author

Justin - I'm sorry, I am surely confusing the issue since I am using the managed object adapter as it stands in Mantle 1.5 today. In the 1.5 tag, -managedObjectKeyForKey: is used a number of times to map between the property and managed object key. Per the documentation for + managedObjectKeysByPropertyKey:

// Any property keys not present in the dictionary are assumed to match the
// entity key that should be used.

So, the key returned by -managedObjectKeyForKey: would be the property key unless specifically associated with NSNull by implementing a null mapping in +managedObjectKeysByPropertyKey:. In my use of Mantle, all of my models' keys map to the managed objects' keys directly. Now, I want to merge (I'm using Mantle/Mantle#351 (comment)) new data into an existing managed object (e.g., I initially only get a foreign key reference from one object GET, then I later do a GET to obtain the associated object and fill in the rest of its attributes). To get this to work, I have to do this:

+ (NSDictionary *)managedObjectKeysByPropertyKey {
    return [[NSDictionary alloc] initWithObjects:[self JSONKeyPathsByPropertyKey].allKeys forKeys:[self JSONKeyPathsByPropertyKey].allKeys];
}

My point was that if unspecified managed object keys are assumed to match the property key by default, we should merge on that default property key as well.

All that being said, it looks like the documentation here for +managedObjectKeysByPropertyKey: says:

// Any keys omitted will not participate in managed object serialization.

So, it looks like that default mapping has been intentionally removed, in which case this issue could be closed here. It's still an (minor) issue in the 1.5 tag, but I wouldn't necessarily bother with it there if it's coming out soon.

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

No branches or pull requests

2 participants