Merge pull request #116 from ikesyo/rawrepresentable-safe-mapping

Do not use force unwrapping for RawRepresentable mapping
tristanhimmelman · Apr 30, 2015 · c098352 · c098352
2 parents c9d8ea6 + 28bdf17
commit c098352
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 93 deletions.
diff --git a/ObjectMapper/Core/FromJSON.swift b/ObjectMapper/Core/FromJSON.swift
@@ -29,81 +29,45 @@ internal final class FromJSON {
 		}
 	}
 
-	/// Raw representable
-	class func rawRepresentable<N: RawRepresentable>(inout field: N, object: N.RawValue?) {
-		if let value = object {
-			field = N(rawValue: value)!
-		}
-	}
-
-	/// Optional raw representable
-	class func rawRepresentable<N: RawRepresentable>(inout field: N?, object: N.RawValue?) {
-		if let value = object {
-			field = N(rawValue: value)
-		}
-	}
-
-	/// Implicitly unwrapped optional basic type
-	class func rawRepresentable<N: RawRepresentable>(inout field: N!, object: N.RawValue?) {
-		if let value = object {
-			field = N(rawValue: value)
-		}
-	}
-
 	/// Array of Raw representable
 	class func rawRepresentableArray<N: RawRepresentable>(inout field: [N], object: [N.RawValue]?) {
 		if let values = object {
-			field = values.map { (v: N.RawValue) in	N(rawValue: v)!	}
+			field = values.filterMap { N(rawValue: $0) }
 		}
 	}
 
 	/// Array of Raw representable
 	class func rawRepresentableArray<N: RawRepresentable>(inout field: [N]?, object: [N.RawValue]?) {
 		if let values = object {
-			field = values.map { (v: N.RawValue) in	N(rawValue: v)!	}
+			field = values.filterMap { N(rawValue: $0) }
 		}
 	}
 
 	/// Array of Raw representable
 	class func rawRepresentableArray<N: RawRepresentable>(inout field: [N]!, object: [N.RawValue]?) {
 		if let values = object {
-			field = values.map { (v: N.RawValue) in	N(rawValue: v)!	}
+			field = values.filterMap { N(rawValue: $0) }
 		}
 	}
 
 	/// Dictionary of Raw representable
 	class func rawRepresentableDict<N: RawRepresentable>(inout field: [String: N], object: [String: N.RawValue]?) {
 		if let values = object {
-			field = map(values) { (k: String, v: N.RawValue) in	(k, N(rawValue: v)!) }
-				.reduce([:]) { (var d, e) in
-					let (k, v) = e
-					d[k] = v
-					return d
-			}
+			field = values.filterMap { N(rawValue: $0) }
 		}
 	}
 
 	/// Dictionary of Raw representable
 	class func rawRepresentableDict<N: RawRepresentable>(inout field: [String: N]?, object: [String: N.RawValue]?) {
 		if let values = object {
-			field = map(values) { (k: String, v: N.RawValue) in	(k, N(rawValue: v)!) }
-				.reduce([:]) { (var d, e) in
-					var (k, v) = e
-					d?[k] = v
-					return d
-			}
+			field = values.filterMap { N(rawValue: $0) }
 		}
 	}
 
 	/// Dictionary of Raw representable
 	class func rawRepresentableDict<N: RawRepresentable>(inout field: [String: N]!, object: [String: N.RawValue]?) {
 		if let values = object {
-			field = map(values) { (k: String, v: N.RawValue) in	(k, N(rawValue: v)!) }
-				.reduce([:]) { (var d, e) in
-					let (k, v) = e
-					d[k] = v
-					return d
-			}
+			field = values.filterMap { N(rawValue: $0) }
 		}
 	}
 

diff --git a/ObjectMapper/Core/Mapper.swift b/ObjectMapper/Core/Mapper.swift
@@ -228,13 +228,8 @@ public final class Mapper<N: Mappable> {
 	* Maps an array of JSON dictionary to an array of object that conforms to Mappable
 	*/
 	public func mapArray(JSONArray: [[String : AnyObject]]) -> [N] {
-		return JSONArray.reduce([]) { (var values, JSON) in
-			// map every element in JSON array to type N
-			if let value = self.map(JSON) {
-				values.append(value)
-			}
-			return values
-		}
+		// map every element in JSON array to type N
+		return JSONArray.filterMap(map)
 	}
 
 	/** Maps a JSON object to a dictionary of Mappable objects if it is a JSON
@@ -252,15 +247,8 @@ public final class Mapper<N: Mappable> {
 	* Maps a JSON dictionary of dictionaries to a dictionary of objects that conform to Mappable.
 	*/
 	public func mapDictionary(JSONDictionary: [String : [String : AnyObject]]) -> [String : N] {
-		return reduce(JSONDictionary, [String: N]()) { (var values, element) in
-			let (key, value) = element
-
-			// map every value in dictionary to type N
-			if let newValue = self.map(value) {
-				values[key] = newValue
-			}
-			return values
-		}
+		// map every value in dictionary to type N
+		return JSONDictionary.filterMap(map)
 	}
 
 	// MARK: Functions that create JSON from objects
@@ -352,8 +340,22 @@ public final class Mapper<N: Mappable> {
 	}
 }
 
+extension Array {
+	internal func filterMap<U>(@noescape f: T -> U?) -> [U] {
+		var mapped = [U]()
+
+		for value in self {
+			if let newValue = f(value) {
+				mapped.append(newValue)
+			}
+		}
+
+		return mapped
+	}
+}
+
 extension Dictionary {
-	private func map<K: Hashable, V>(f: Element -> (K, V)) -> [K : V] {
+	internal func map<K: Hashable, V>(@noescape f: Element -> (K, V)) -> [K : V] {
 		var mapped = [K : V]()
 
 		for element in self {
@@ -363,4 +365,15 @@ extension Dictionary {
 
 		return mapped
 	}
+
+	internal func filterMap<U>(@noescape f: Value -> U?) -> [Key : U] {
+		var mapped = [Key : U]()
+
+		for (key, value) in self {
+			let newValue = f(value)
+			mapped[key] = newValue
+		}
+
+		return mapped
+	}
 }
diff --git a/ObjectMapper/Core/Operators.swift b/ObjectMapper/Core/Operators.swift
@@ -54,33 +54,21 @@ public func <- <T>(inout left: T!, right: Map) {
 * Object of Raw Representable type
 */
 public func <- <T: RawRepresentable>(inout left: T, right: Map) {
-	if right.mappingType == MappingType.FromJSON {
-		FromJSON.rawRepresentable(&left, object: right.value())
-	} else {
-		ToJSON.rawRepresentable(left, key: right.currentKey!, dictionary: &right.JSONDictionary)
-	}
+	left <- (right, EnumTransform())
 }
 
 /**
 * Optional Object of Raw Representable type
 */
 public func <- <T: RawRepresentable>(inout left: T?, right: Map) {
-	if right.mappingType == MappingType.FromJSON {
-		FromJSON.rawRepresentable(&left, object: right.value())
-	} else {
-		ToJSON.rawRepresentable(left, key: right.currentKey!, dictionary: &right.JSONDictionary)
-	}
+	left <- (right, EnumTransform())
 }
 
 /**
 * Implicitly Unwrapped Optional Object of Raw Representable type
 */
 public func <- <T: RawRepresentable>(inout left: T!, right: Map) {
-	if right.mappingType == MappingType.FromJSON {
-		FromJSON.rawRepresentable(&left, object: right.value())
-	} else {
-		ToJSON.rawRepresentable(left, key: right.currentKey!, dictionary: &right.JSONDictionary)
-	}
+	left <- (right, EnumTransform())
 }
 
 // MARK:- Arrays of Raw Representable type

diff --git a/ObjectMapper/Core/ToJSON.swift b/ObjectMapper/Core/ToJSON.swift
@@ -99,16 +99,6 @@ internal final class ToJSON {
         }
     }
 
-	class func rawRepresentable<N: RawRepresentable>(field: N, key: String, inout dictionary: [String : AnyObject]) {
-		basicType(field.rawValue, key: key, dictionary: &dictionary)
-	}
-
-  	class func rawRepresentable<N: RawRepresentable>(field: N?, key: String, inout dictionary: [String : AnyObject]) {
-			if let field = field {
-				rawRepresentable(field, key: key, dictionary: &dictionary)
-			}
-	  }
-
 	class func rawRepresentableArray<N: RawRepresentable>(field: [N], key: String, inout dictionary: [String : AnyObject]) {
 		basicType(field.map { e in e.rawValue }, key: key, dictionary: &dictionary)
 	}
@@ -120,11 +110,8 @@ internal final class ToJSON {
 	}
 
 	class func rawRepresentableDict<N: RawRepresentable>(field: [String: N], key: String, inout dictionary: [String : AnyObject]) {
-		let raw: [String: N.RawValue] = map(field) { (k, v) in (k, v.rawValue) }
-			.reduce([:]) { (var d, e) in
-				let (k, v) = e
-				d[k] = v
-				return d
+		let raw: [String: N.RawValue] = field.map { key, value in
+			return (key, value.rawValue)
 		}
 		basicType(raw, key: key, dictionary: &dictionary)
 	}

diff --git a/ObjectMapper/Transforms/EnumTransform.swift b/ObjectMapper/Transforms/EnumTransform.swift
@@ -14,16 +14,16 @@ public class EnumTransform<T: RawRepresentable>: TransformType {
 
 	public init() {}
 
-	public func transformFromJSON(value: AnyObject?) -> Object? {
+	public func transformFromJSON(value: AnyObject?) -> T? {
 		if let raw = value as? T.RawValue {
-			return T(rawValue: raw) ?? nil
+			return T(rawValue: raw)
 		}
 		return nil
 	}
 
-	public func transformToJSON(value: Object?) -> JSON? {
+	public func transformToJSON(value: T?) -> T.RawValue? {
 		if let obj = value {
-			return obj.rawValue as T.RawValue
+			return obj.rawValue
 		}
 		return nil
 	}

diff --git a/ObjectMapperTests/BasicTypesTestsFromJSON.swift b/ObjectMapperTests/BasicTypesTestsFromJSON.swift
@@ -282,6 +282,17 @@ class BasicTypesTestsFromJSON: XCTestCase {
 		expect(mappedObject?.enumIntImplicitlyUnwrapped).to(equal(value))
 	}
 
+	func testMappingIntEnumFromJSONShouldNotCrashWithNonDefinedvalue() {
+		let value = Int.min
+		let JSONString = "{\"enumInt\" : \(value), \"enumIntOpt\" : \(value), \"enumIntImp\" : \(value) }"
+
+		let mappedObject = mapper.map(JSONString)
+		expect(mappedObject).notTo(beNil())
+		expect(mappedObject?.enumInt).to(equal(BasicTypes.EnumInt.Default))
+		expect(mappedObject?.enumIntOptional).to(beNil())
+		expect(mappedObject?.enumIntImplicitlyUnwrapped).to(beNil())
+	}
+
 	func testMappingDoubleEnumFromJSON(){
 		var value: BasicTypes.EnumDouble = .Another
 		let JSONString = "{\"enumDouble\" : \(value.rawValue), \"enumDoubleOpt\" : \(value.rawValue), \"enumDoubleImp\" : \(value.rawValue) }"
@@ -326,6 +337,17 @@ class BasicTypesTestsFromJSON: XCTestCase {
 		expect(mappedObject?.arrayEnumIntImplicitlyUnwrapped?.first).to(equal(value))
 	}
 
+	func testMappingEnumIntArrayFromJSONShouldNotCrashWithNonDefinedvalue() {
+		let value = Int.min
+		let JSONString = "{ \"arrayEnumInt\" : [\(value)], \"arrayEnumIntOpt\" : [\(value)], \"arrayEnumIntImp\" : [\(value)] }"
+
+		var mappedObject = mapper.map(JSONString)
+		expect(mappedObject).notTo(beNil())
+		expect(mappedObject?.arrayEnumInt.first).to(beNil())
+		expect(mappedObject?.arrayEnumIntOptional?.first).to(beNil())
+		expect(mappedObject?.arrayEnumIntImplicitlyUnwrapped?.first).to(beNil())
+	}
+
 	func testMappingEnumIntDictionaryFromJSON(){
 		let key = "key"
 		let value: BasicTypes.EnumInt = .Another
@@ -338,6 +360,18 @@ class BasicTypesTestsFromJSON: XCTestCase {
 		expect(mappedObject?.dictEnumIntImplicitlyUnwrapped?[key]).to(equal(value))
 	}
 
+	func testMappingEnumIntDictionaryFromJSONShouldNotCrashWithNonDefinedvalue() {
+		let key = "key"
+		let value = Int.min
+		let JSONString = "{ \"dictEnumInt\" : { \"\(key)\" : \(value) }, \"dictEnumIntOpt\" : { \"\(key)\" : \(value) }, \"dictEnumIntImp\" : { \"\(key)\" : \(value) } }"
+
+		let mappedObject = mapper.map(JSONString)
+		expect(mappedObject).notTo(beNil())
+		expect(mappedObject?.arrayEnumInt.first).to(beNil())
+		expect(mappedObject?.dictEnumIntOptional?[key]).to(beNil())
+		expect(mappedObject?.dictEnumIntImplicitlyUnwrapped?[key]).to(beNil())
+	}
+
 	func testObjectModelOptionalDictionnaryOfPrimitives() {
 		let JSON: [String: [String: AnyObject]] = ["dictStringString":["string": "string"], "dictStringBool":["string": false], "dictStringInt":["string": 1], "dictStringDouble":["string": 1.1], "dictStringFloat":["string": 1.2]]