Skip to content

MR_importValue does not honor the return value of callbacks #644

Closed
eflath opened this Issue Jan 9, 2014 · 3 comments

2 participants

@eflath
eflath commented Jan 9, 2014

I discovered this feature via a blog post by Saul Mora (scroll down to "import<;attributeName>;:"). The idea is that I should be able to intervene during the import process to perform any custom tasks related to the import. As for the return value, Mora has this to say:

"
You’ll also notice that this method returns a boolean. This tells MagicalImport that your custom code handled the import. Return a YES if your code processed the data, so that MagicalRecord won’t. This is also the way to skip importing a particular entity, though this behavior is not encouraged. And, conversely, return NO if you want MagicalImport to continue processing the attribute and use the default import routines.
"

However, the MR_importValue method on NSManagedObject (MagicalRecord_DataImport) does not honor the return value of the import<;attributeName>;: callback. Instead, it assumes the custom implementation has handled the import.

- (BOOL) MR_importValue:(id)value forKey:(NSString *)key
{
    NSString *selectorString = [NSString stringWithFormat:@"import%@:", [key MR_capitalizedFirstCharacterString]];
    SEL selector = NSSelectorFromString(selectorString);
    if ([self respondsToSelector:selector])
    {
        NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:[self methodSignatureForSelector:selector]];
        [invocation setTarget:self];
        [invocation setSelector:selector];
        [invocation setArgument:&value atIndex:2];
        [invocation invoke];
        // Should this really return YES!?
        return YES;
    }
    return NO;
}

Rather than returning YES on the line in question, it seems to me that we should be returning the return value of the invocation. This would allow the callback to function as described.

@eflath
eflath commented Jan 9, 2014

possible fix in #645

@casademora
Magical Panda Software member
@eflath
eflath commented Jan 9, 2014

One concern is that if the user is returning void from their custom import method, that would evaluate to NO and MagicalImport would think it needed to proceed with the default import routine. This could break an existing implementation that was relying on this little quirk in which YES was the assumed return value

@tonyarnold tonyarnold closed this in 916f167 Apr 8, 2014
@naqi naqi added a commit to naqi/MagicalRecord that referenced this issue Apr 7, 2016
@tonyarnold tonyarnold Honour the return value from `-(BOOL)import<#AttributeName#>:` data i…
…mport methods

Closes #644, #645
040ba8e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.