Fix for crash based on set nil for missing element policy. Added spec…

…s covering the crash. Merging directly to 0.9.1 and tagging as 0.9.1.1
jwang · Mar 29, 2011 · 9ed0621 · 9ed0621
1 parent 1d50d07
commit 9ed0621
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 6 deletions.
diff --git a/Code/ObjectMapping/RKObjectMapper.m b/Code/ObjectMapping/RKObjectMapper.m
@@ -346,7 +346,7 @@ - (void)updateModel:(NSObject<RKObjectMappable>*)model ifNewPropertyValue:(id)pr
 - (void)setPropertiesOfModel:(NSObject<RKObjectMappable>*)model fromElements:(NSDictionary*)elements {
 	NSDictionary* elementToPropertyMappings = [self elementToPropertyMappingsForModel:model];
 	for (NSString* elementKeyPath in elementToPropertyMappings) {		
-		id elementValue = nil;		
+		id elementValue = nil;
 		BOOL setValue = YES;
 
 		@try {
@@ -357,8 +357,7 @@ - (void)setPropertiesOfModel:(NSObject<RKObjectMappable>*)model fromElements:(NS
 			setValue = NO;
 		}
 
-		// TODO: Need a way to differentiate between a keyPath that exists, but contains a nil
-		// value and one that is not present in the payload. Causes annoying problems!
+		// nil is returned when the collection does not contain the element
 		if (nil == elementValue) {
 			setValue = (_missingElementMappingPolicy == RKSetNilForMissingElementMappingPolicy);
 		}
@@ -374,8 +373,9 @@ - (void)setPropertiesOfModel:(NSObject<RKObjectMappable>*)model fromElements:(NS
 				}
 			}
 
-            // If we know the destination class, the property is not of the correct class, and the property value is not null...
-            if (class && ![propertyValue isKindOfClass:class] && ![propertyValue isKindOfClass:[NSNull class]]) {
+            // If we know the destination class, the property is not of the correct class, and the property value is not null or nil...
+            // if (class && propertyValue && ![propertyValue isKindOfClass:class] && ![propertyValue isEqual:[NSNull null]]) {
+            if (class && propertyValue && ![propertyValue isKindOfClass:class] && ![propertyValue isEqual:[NSNull null]]) {
                 // Then we must cooerce the element (probably a string) into the correct class.
                 // Currently this only supports NSNumbers (NSDates are handled above).
                 // New cooersions will be added on an as-needed basis.

diff --git a/Examples/RKDiscussionBoardExample/DiscussionBoard/DiscussionBoard.xcodeproj/project.pbxproj b/Examples/RKDiscussionBoardExample/DiscussionBoard/DiscussionBoard.xcodeproj/project.pbxproj
@@ -70,6 +70,13 @@
 			remoteGlobalIDString = 73057FD11331AD2E001908EE;
 			remoteInfo = "RestKitJSONParser+JSONKit";
 		};
+		25A7B1DA13421D28004816AA /* PBXContainerItemProxy */ = {
+			isa = PBXContainerItemProxy;
+			containerPortal = A7A2D39412D7822C00683D6F /* RestKit.xcodeproj */;
+			proxyType = 2;
+			remoteGlobalIDString = 25BD43BD1340315800DBACDD;
+			remoteInfo = "RestKitXMLParser+Libxml";
+		};
 		3F2562F812DBA4C800AFD2D1 /* PBXContainerItemProxy */ = {
 			isa = PBXContainerItemProxy;
 			containerPortal = A7A2D39412D7822C00683D6F /* RestKit.xcodeproj */;
@@ -1081,6 +1088,7 @@
 				A7A2D3B112D7822D00683D6F /* libRestKitJSONParserYAJL.a */,
 				A7A2D3B312D7822D00683D6F /* libRestKitJSONParserSBJSON.a */,
 				25825AF41337CEAC00C0E3D2 /* libRestKitJSONParserJSONKit.a */,
+				25A7B1DB13421D28004816AA /* libRestKitXMLParserLibxml.a */,
 				A7A2D3B512D7822D00683D6F /* libRestKitCoreData.a */,
 				A7A2D3B712D7822D00683D6F /* libRestKitThree20.a */,
 				A7A2D3B912D7822D00683D6F /* UISpec.app */,
@@ -1152,6 +1160,13 @@
 			remoteRef = 25825AF31337CEAC00C0E3D2 /* PBXContainerItemProxy */;
 			sourceTree = BUILT_PRODUCTS_DIR;
 		};
+		25A7B1DB13421D28004816AA /* libRestKitXMLParserLibxml.a */ = {
+			isa = PBXReferenceProxy;
+			fileType = archive.ar;
+			path = libRestKitXMLParserLibxml.a;
+			remoteRef = 25A7B1DA13421D28004816AA /* PBXContainerItemProxy */;
+			sourceTree = BUILT_PRODUCTS_DIR;
+		};
 		A7A2D3AB12D7822D00683D6F /* libRestKitNetwork.a */ = {
 			isa = PBXReferenceProxy;
 			fileType = archive.ar;

diff --git a/RestKit.xcodeproj/xcshareddata/xcschemes/UISpec.xcscheme b/RestKit.xcodeproj/xcshareddata/xcschemes/UISpec.xcscheme
@@ -61,14 +61,19 @@
       <EnvironmentVariables>
          <EnvironmentVariable
             key = "UISPEC_SPEC"
-            value = "RKClientSpec"
+            value = "RKObjectMapperSpec"
             isEnabled = "NO">
          </EnvironmentVariable>
          <EnvironmentVariable
             key = "RESTKIT_IP_ADDRESS"
             value = "10.0.1.4"
             isEnabled = "NO">
          </EnvironmentVariable>
+         <EnvironmentVariable
+            key = "UISPEC_EXAMPLE"
+            value = "itShouldNotCrashWhenAttemptingToMapWithAMissingElement"
+            isEnabled = "NO">
+         </EnvironmentVariable>
       </EnvironmentVariables>
       <AdditionalOptions>
       </AdditionalOptions>

diff --git a/Specs/ObjectMapping/RKObjectMapperSpec.m b/Specs/ObjectMapping/RKObjectMapperSpec.m
@@ -156,6 +156,55 @@ - (void)itShouldMapAnArrayOfHashes {
     [expectThat([[array lastObject] name]) should:be(@"mimi")];
 }
 
+// NOTE: email attribute is omitted!
+- (NSString*)userJSONMissingAnElement {
+    return @"{\"user\":"
+    @"{\"encrypted_password\":\"68dad82a867c4a61719fec594c119188ed35cd3b7d42eed1647e46d85f2ffdd8\",\"name\":\"mimi\","
+    @"\"salt\":\"809c79c24cbebfe3f9feea2e9bf98255e5af0f0b8514b6c0d71dcc63fa083688\",\"created_at\":\"2011-02-16T23:04:22Z\","
+    @"\"updated_at\":\"2011-02-16T23:04:22Z\",\"id\":"
+    @"10,\"image\":null}}";
+}
+
+- (void)itShouldNotCrashWhenAttemptingToMapWithAMissingElement {
+    RKObjectMapper* mapper = [[RKObjectMapper alloc] init];
+    mapper.missingElementMappingPolicy = RKSetNilForMissingElementMappingPolicy;
+	mapper.format = RKMappingFormatJSON;
+    [mapper registerClass:[RKObjectMapperSpecUser class] forElementNamed:@"user"];
+
+    NSException* exception = nil;
+    @try {
+        [mapper mapFromString:[self userJSONMissingAnElement]];
+    }
+    @catch (NSException * e) {
+        exception = e;
+    }
+    [expectThat(exception) should:be(nil)];
+}
+
+- (void)itShouldNotNilOutWhenAttemptingToMapWithAMissingElementIfConfiguredNotTo {
+    RKObjectMapperSpecUser* user = [RKObjectMapperSpecUser object];
+    user.email = @"foo@bar.com";
+    RKObjectMapper* mapper = [[RKObjectMapper alloc] init];
+    mapper.missingElementMappingPolicy = RKIgnoreMissingElementMappingPolicy;
+	mapper.format = RKMappingFormatJSON;
+    [mapper registerClass:[RKObjectMapperSpecUser class] forElementNamed:@"user"];
+
+    [mapper mapObject:user fromString:[self userJSONMissingAnElement]];
+    [expectThat(user.email) should:be(@"foo@bar.com")];
+}
+
+- (void)itShouldNilOutWhenAttemptingToMapWithAMissingElementIfConfiguredToDoSo {
+    RKObjectMapperSpecUser* user = [RKObjectMapperSpecUser object];
+    user.email = @"foo@bar.com";
+    RKObjectMapper* mapper = [[RKObjectMapper alloc] init];
+    mapper.missingElementMappingPolicy = RKSetNilForMissingElementMappingPolicy;
+	mapper.format = RKMappingFormatJSON;
+    [mapper registerClass:[RKObjectMapperSpecUser class] forElementNamed:@"user"];
+
+    [mapper mapObject:user fromString:[self userJSONMissingAnElement]];
+    [expectThat(user.email) should:be(nil)];
+}
+
 - (void)itShouldMapAnArrayOfStringsToAProperty {
 /*
  'tags' =>