[RKManagedObjectThreadSafeInvocation serializeManagedObjectsForArgument:withKeyPaths:] is not properly serializing managed objects #709

Closed
echamberlain opened this Issue May 1, 2012 · 4 comments

Comments

Projects
None yet
4 participants

Hello,

serializeManagedObjectsForArgument:withKeyPaths: is not properly serializing managed objects. Instead of passing managedObject objectID's between the threads, the method passes the actual managedObjects.

To reproduce this bug, add the following code to your objectLoader delegate:


- (void)objectLoader:(RKObjectLoader *)objectLoader didLoadObjects:(NSArray *)objects {
    for (id object in objects) {
        if ([object isKindOfClass:[NSManagedObject class]]) {
            NSManagedObject *managedObject = (NSManagedObject *)object;
            NSAssert([managedObject.managedObjectContext isEqual:[[RKManagedObjectStore defaultObjectStore] managedObjectContextForCurrentThread]], @"The managedObjectContexts should be the same");
        }
    }
}

The managed objects should be in the same managed object context as the managedObjectContextForCurrentThread.

The problem appears to be in [RKManagedObjectThreadSafeInvocation serializeManagedObjectsForArgument:withKeyPaths:].


- (void)serializeManagedObjectsForArgument:(id)argument withKeyPaths:(NSSet*)keyPaths {
    for (NSString* keyPath in keyPaths) {
        id value = [argument valueForKeyPath:keyPath];
        if ([value isKindOfClass:[NSManagedObject class]]) {
            [argument setValue:[(NSManagedObject*)value objectID] forKeyPath:keyPath];
        } else if ([value respondsToSelector:@selector(allObjects)]) {
            id collection = [[[[[value class] alloc] init] autorelease] mutableCopy];
            for (id subObject in value) {
                if ([subObject isKindOfClass:[NSManagedObject class]]) {
                    [collection addObject:[(NSManagedObject*)subObject objectID]];
                } else {
                    [collection addObject:subObject];
                }
            }

            [argument setValue:collection forKeyPath:keyPath];
//            NSAssert([collection isEqual:[argument valueForKeyPath:keyPath]], @"these should be equal");
            [collection release];
        }
    }
}

Uncomment the NSAssert I've added and the method with raise an exception.

I traced the problem to the use of setValue:forKeyPath:, when called on argument, the value isn't actually set. The workaround is to use setValue:forKey: instead.

In our failure case, the keyPath is @"data.countries".

Here's our workaround code:


- (void)setValue:(id)value forKeyPathOrKey:(NSString *)keyPath object:(id)object {
    [object setValue:value forKeyPath:keyPath];

    id testValue = [object valueForKeyPath:keyPath];

    if (![value isEqual:testValue]) {
        [object setValue:value forKey:keyPath];

        testValue = [object valueForKeyPath:keyPath];

        NSAssert([value isEqual:testValue], @"Could not set value");
    }
}

- (void)serializeManagedObjectsForArgument:(id)argument withKeyPaths:(NSSet*)keyPaths {
    for (NSString* keyPath in keyPaths) {
        id value = [argument valueForKeyPath:keyPath];
        if ([value isKindOfClass:[NSManagedObject class]]) {
            NSManagedObjectID *objectID = [(NSManagedObject*)value objectID];
            [self setValue:objectID forKeyPathOrKey:keyPath object:argument];
        } else if ([value respondsToSelector:@selector(allObjects)]) {
            id collection = [[[[[value class] alloc] init] autorelease] mutableCopy];
            for (id subObject in value) {
                if ([subObject isKindOfClass:[NSManagedObject class]]) {
                    [collection addObject:[(NSManagedObject*)subObject objectID]];
                } else {
                    [collection addObject:subObject];
                }
            }

            [self setValue:collection forKeyPathOrKey:keyPath object:argument];
            [collection release];
        }
    }
}

- (void)deserializeManagedObjectIDsForArgument:(id)argument withKeyPaths:(NSSet*)keyPaths {   
    for (NSString* keyPath in keyPaths) {
        id value = [argument valueForKeyPath:keyPath];
        if ([value isKindOfClass:[NSManagedObjectID class]]) {
            NSAssert(self.objectStore, @"Object store cannot be nil");
            NSManagedObject* managedObject = [self.objectStore objectWithID:(NSManagedObjectID*)value];
            NSAssert(managedObject, @"Expected managed object for ID %@, got nil", value);
            [self setValue:managedObject forKeyPathOrKey:keyPath object:argument];
        } else if ([value respondsToSelector:@selector(allObjects)]) {
            id collection = [[[[[value class] alloc] init] autorelease] mutableCopy];
            for (id subObject in value) {
                if ([subObject isKindOfClass:[NSManagedObjectID class]]) {
                    NSAssert(self.objectStore, @"Object store cannot be nil");
                    NSManagedObject* managedObject = [self.objectStore objectWithID:(NSManagedObjectID*)subObject];
                    [collection addObject:managedObject];
                } else {
                    [collection addObject:subObject];
                }
            }

            [self setValue:collection forKeyPathOrKey:keyPath object:argument];
            [collection release];
        }
    }
}

The proposed solution doesn't work for my case. The argument in serializeManagedObjectsForArgument: is a dictionary of
{
FolderSync = "statusCode:1";
"FolderSync.Changes.Add" = (
"name:Trash, type:4, id:3 parent:0",
"name:Drafts, type:3, id:4 parent:0",
"name:First Folder, type:12, id:5 parent:0",
"name:Inbox, type:2, id:6 parent:0",
"name:Junk E-Mail, type:12, id:8 parent:0",
"name:Outbox, type:6, id:10 parent:0",
"name:Sent, type:5, id:11 parent:0"
);
}

I get a crash executing [object setValue:value forKeyPath:keyPath]; in setValue:forKeyPathOrKey:object: where value is an array of objectIDs and keyPath is "FolderSync.Changes.Add".

*** Terminating app due to uncaught exception 'NSUnknownKeyException', reason: '[<PKStatus 0x81a8090> valueForUndefinedKey:]: this class is not key value coding-compliant for the key Changes.'

I believe the solution is much simpler. The keys of the dictionary are strings that represent key paths in the response object but are not key paths into the dictionary. Therefore the valueForKeyPaths should be replaced with valueForKey and the setValue:forKeyPath:object replaced with setValue:forKey. The method setValue:forKeyPath:object can then be removed.

Here the solution that works for me.

- (void)serializeManagedObjectsForArgument:(id)argument withKeyPaths:(NSSet*)keyPaths {
    for (NSString* keyPath in keyPaths) {
        id value = [argument valueForKey:keyPath];
        if ([value isKindOfClass:[NSManagedObject class]]) {
            NSManagedObjectID *objectID = [(NSManagedObject*)value objectID];
            [argument setValue:objectID forKey:keyPath];
        } else if ([value respondsToSelector:@selector(allObjects)]) {
            id collection = [[[[[value class] alloc] init] autorelease] mutableCopy];
            for (id subObject in value) {
                if ([subObject isKindOfClass:[NSManagedObject class]]) {
                    [collection addObject:[(NSManagedObject*)subObject objectID]];
                } else {
                    [collection addObject:subObject];
                }
            }

            [argument setValue:collection forKey:keyPath];
            [collection release];
        }
    }
}

- (void)deserializeManagedObjectIDsForArgument:(id)argument withKeyPaths:(NSSet*)keyPaths {
    for (NSString* keyPath in keyPaths) {
        id value = [argument valueForKey:keyPath];
        if ([value isKindOfClass:[NSManagedObjectID class]]) {
            NSAssert(self.objectStore, @"Object store cannot be nil");
            NSManagedObject* managedObject = [self.objectStore objectWithID:(NSManagedObjectID*)value];
            NSAssert(managedObject, @"Expected managed object for ID %@, got nil", value);
            [argument setValue:managedObject forKey:keyPath];
        } else if ([value respondsToSelector:@selector(allObjects)]) {
            id collection = [[[[[value class] alloc] init] autorelease] mutableCopy];
            for (id subObject in value) {
                if ([subObject isKindOfClass:[NSManagedObjectID class]]) {
                    NSAssert(self.objectStore, @"Object store cannot be nil");
                    NSManagedObject* managedObject = [self.objectStore objectWithID:(NSManagedObjectID*)subObject];
                    [collection addObject:managedObject];
                } else {
                    [collection addObject:subObject];
                }
            }

            [argument setValue:collection forKey:keyPath];
            [collection release];
        }
    }
}

preston commented Jul 18, 2012

I seem to be having this same issue. Will this be pulled into trunk?

Owner

blakewatters commented Oct 29, 2012

I have eliminated this class on development.

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