From 72fb80e07d307f51e28d4e0fa02aa5cc986de1a1 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Fri, 19 Feb 2016 08:44:59 -0800 Subject: [PATCH] Add compile time safety for all types Previously Mapper worked by conditionally downcasting values to the expected types. For example: ``` let string: String? = map.optionalFrom("field") ``` Would basically translate to: ``` JSON["field"] as? String? ``` This worked perfectly for many common types, such as `String` and `Int`. But this became a problem when you used types that could not be converted like this such as `NSDate`. Previously this line of code would compile without any additions to Mapper: ``` let date: NSDate? = map.optionalFrom("field") ``` But since this would translate to: ``` JSON["field"] as? NSDate? ``` This would fail 100% of the time. But since it compiled it would leave callers assuming that it could work. This is no longer the case. Now if you attempt to cast to a type that doesn't conform to `Convertible` (unless it's `RawRepresentable` or you're using a transformation) it will fail to compile. There are some adverse side effects of this change. For one, you can no longer map to `AnyObject` `[AnyObject]` or `[String: AnyObject]`. This is because if we made `AnyObject` conform to `DefaultConvertible` we would be in the same place we were before with the `NSDate` example compiling, even though it shouldn't. The alternative in the latter 2 cases is to use `NSArray` and `NSDictionary` respectively. After you map to those, you could then map them back to the Swift types if it was necessary. Another possible solution for all 3 cases is to create a transformation that returns these types using `as?`. This way you won't break the compile time safety for all other types, but you can still get the types using `AnyObject` back. --- .swiftlint.yml | 1 + CHANGELOG.md | 3 + Mapper.xcodeproj/project.pbxproj | 4 + Sources/DefaultConvertible.swift | 37 +++++++ Sources/Mapper.swift | 122 +++++++++++++---------- Tests/Mapper/ConvertibleValueTests.swift | 84 ++++++++++++++++ Tests/Mapper/ErrorTests.swift | 8 +- Tests/Mapper/NormalValueTests.swift | 26 ++++- 8 files changed, 227 insertions(+), 58 deletions(-) create mode 100644 Sources/DefaultConvertible.swift diff --git a/.swiftlint.yml b/.swiftlint.yml index 10b62d2..46469d8 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -1,6 +1,7 @@ line_length: - 110 disabled_rules: + - file_length - force_cast - force_try - nesting diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d4d56e..4979d95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Breaking +- Require types to be `Convertible` in order to use them. + [Keith Smiley](https://github.com/keith) + [#59](https://github.com/lyft/mapper/pull/59) - Allow transformations to throw when the field is missing [Keith Smiley](https://github.com/keith) [#52](https://github.com/lyft/mapper/pull/52) diff --git a/Mapper.xcodeproj/project.pbxproj b/Mapper.xcodeproj/project.pbxproj index e9b5705..af1134e 100644 --- a/Mapper.xcodeproj/project.pbxproj +++ b/Mapper.xcodeproj/project.pbxproj @@ -8,6 +8,7 @@ /* Begin PBXBuildFile section */ C201748E1BD5509D00E4FE18 /* Mapper.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = C20174831BD5509D00E4FE18 /* Mapper.framework */; }; + C20586EC1CDEAD9900658A67 /* DefaultConvertible.swift in Sources */ = {isa = PBXBuildFile; fileRef = C20586EB1CDEAD9900658A67 /* DefaultConvertible.swift */; }; C2B977A91CCD7AD800FDA451 /* ErrorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C2B977A71CCD7AB200FDA451 /* ErrorTests.swift */; }; C2C036FA1C2B1A0B003FB853 /* Convertible.swift in Sources */ = {isa = PBXBuildFile; fileRef = C2C036F31C2B1A0B003FB853 /* Convertible.swift */; }; C2C036FB1C2B1A0B003FB853 /* MapperError.swift in Sources */ = {isa = PBXBuildFile; fileRef = C2C036F41C2B1A0B003FB853 /* MapperError.swift */; }; @@ -39,6 +40,7 @@ /* Begin PBXFileReference section */ C20174831BD5509D00E4FE18 /* Mapper.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Mapper.framework; sourceTree = BUILT_PRODUCTS_DIR; }; C201748D1BD5509D00E4FE18 /* MapperTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = MapperTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; + C20586EB1CDEAD9900658A67 /* DefaultConvertible.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DefaultConvertible.swift; sourceTree = ""; }; C2B977A71CCD7AB200FDA451 /* ErrorTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ErrorTests.swift; sourceTree = ""; }; C2C036D11C2B180D003FB853 /* Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; C2C036D41C2B180D003FB853 /* UniversalFramework_Base.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = UniversalFramework_Base.xcconfig; sourceTree = ""; }; @@ -115,6 +117,7 @@ isa = PBXGroup; children = ( C2C036F31C2B1A0B003FB853 /* Convertible.swift */, + C20586EB1CDEAD9900658A67 /* DefaultConvertible.swift */, C2C036F51C2B1A0B003FB853 /* Mappable.swift */, C2C036F61C2B1A0B003FB853 /* Mapper.swift */, C2C036F41C2B1A0B003FB853 /* MapperError.swift */, @@ -230,6 +233,7 @@ C2C036FD1C2B1A0B003FB853 /* Mapper.swift in Sources */, C2C037001C2B1A0B003FB853 /* Transform.swift in Sources */, C2C036FB1C2B1A0B003FB853 /* MapperError.swift in Sources */, + C20586EC1CDEAD9900658A67 /* DefaultConvertible.swift in Sources */, C2C036FC1C2B1A0B003FB853 /* Mappable.swift in Sources */, C2C036FA1C2B1A0B003FB853 /* Convertible.swift in Sources */, ); diff --git a/Sources/DefaultConvertible.swift b/Sources/DefaultConvertible.swift new file mode 100644 index 0000000..bd38ce9 --- /dev/null +++ b/Sources/DefaultConvertible.swift @@ -0,0 +1,37 @@ +/** + The DefaultConvertible protocol defines values that can be converted from JSON by conditionally downcasting + + This means any value that you could use with `as?`. If you have other types that would work to be casted from + JSON by just using `value as? YourType` you should conform to this protocol in order to get the definition of + that for free. + + The reason this is a separate protocol instead of just using a prtocol extension on Convertible is so other + Consumers of Convertible will still get an error if they don't implement `fromMap` + */ +public protocol DefaultConvertible: Convertible {} + +extension DefaultConvertible { + public static func fromMap(value: AnyObject?) throws -> ConvertedType { + if let object = value as? ConvertedType { + return object + } + + throw MapperError.ConvertibleError(value: value, type: ConvertedType.self) + } +} + +// MARK: - Default Conformances + +/// These Foundation conformances are acceptable since we already depend on Foundation. No other frameworks +/// Should be important as part of Mapper for default conformances. Consumers should conform any other common +/// Types in an extension in their own projects (e.g. `CGFloat`) +import Foundation +extension NSDictionary: DefaultConvertible {} +extension NSArray: DefaultConvertible {} + +extension String: DefaultConvertible {} +extension Int: DefaultConvertible {} +extension UInt: DefaultConvertible {} +extension Float: DefaultConvertible {} +extension Double: DefaultConvertible {} +extension Bool: DefaultConvertible {} diff --git a/Sources/Mapper.swift b/Sources/Mapper.swift index c45d440..652229b 100644 --- a/Sources/Mapper.swift +++ b/Sources/Mapper.swift @@ -17,61 +17,6 @@ public struct Mapper { self.JSON = JSON } - // MARK: - T - - /** - Get a typed value from the given field in the source data - - - parameter field: The field to retrieve from the source data, can be an empty string to return the - entire data set - - - throws: MapperError.MissingFieldError if the field doesn't exist - - throws: MapperError.TypeMismatchError if the value exists with the incorrect type - - - returns: The value for the given field, if it can be converted to the expected type T - */ - @warn_unused_result - public func from(field: String) throws -> T { - let value = try self.JSONFromField(field) - if let value = value as? T { - return value - } - - throw MapperError.TypeMismatchError(field: field, value: value, type: T.self) - } - - /** - Get an optional typed value from the given field in the source data - - - parameter field: The field to retrieve from the source data, can be an empty string to return the - entire data set - - - returns: The value for the given field, if it can be converted to the expected type T otherwise nil - */ - @warn_unused_result - public func optionalFrom(field: String) -> T? { - return try? self.from(field) - } - - /** - Get an optional value from the given fields and source data. This returns the first non-nil value - produced in order based on the array of fields - - - parameter fields: The array of fields to check from the source data. - - - returns: The first non-nil value to be produced from the array of fields, or nil if none exist - */ - @warn_unused_result - public func optionalFrom(fields: [String]) -> T? { - for field in fields { - if let value: T = try? self.from(field) { - return value - } - } - - return nil - } - // MARK: - T: RawRepresentable /** @@ -258,6 +203,25 @@ public struct Mapper { return try self.from(field, transformation: T.fromMap) } + /** + Get a Convertible value from a field in the source data + + This transparently converts your types that conform to Convertible to properties on the Mappable type + + - parameter field: The field to retrieve from the source data, can be an empty string to return the + entire data set + + - throws: Any error produced by the custom Convertible implementation + + - note: This function is necessary because swift does not coerce the from that returns T to an optional + + - returns: The value for the given field, if it can be converted to the expected type Optional + */ + @warn_unused_result + public func from(field: String) throws -> T? { + return try self.from(field, transformation: T.fromMap) + } + /** Get an array of Convertible values from a field in the source data @@ -312,6 +276,54 @@ public struct Mapper { return try? self.from(field) } + /** + Get a dictionary of Convertible values from a field in the source data + + This transparently converts a source dictionary to a dictionary of 2 Convertible types + + - parameter field: The field to retrieve from the source data, can be an empty string to return the + entire data set + + - throws: MapperError.TypeMismatchError if the value for the given field isn't a NSDictionary + - throws: Any error produced by the Convertible implementation of either expected type + + - returns: A dictionary where the keys and values are created using their convertible implementations + */ + @warn_unused_result + public func from(field: String) throws -> [U: T] + { + let object = try self.JSONFromField(field) + guard let data = object as? NSDictionary else { + throw MapperError.TypeMismatchError(field: field, value: object, type: NSDictionary.self) + } + + var result = [U: T]() + for (key, value) in data { + result[try U.fromMap(key)] = try T.fromMap(value) + } + + return result + } + + /** + Get an optional dictionary of Convertible values from a field in the source data + + This transparently converts a source dictionary to a dictionary of 2 Convertible types + + - parameter field: The field to retrieve from the source data, can be an empty string to return the + entire data set + + - returns: A dictionary where the keys and values are created using their convertible implementations or + nil if anything throws + */ + @warn_unused_result + public func optionalFrom(field: String) -> [U: T]? + { + return try? self.from(field) + } + /** Get an optional value from the given fields and source data. This returns the first non-nil value produced in order based on the array of fields diff --git a/Tests/Mapper/ConvertibleValueTests.swift b/Tests/Mapper/ConvertibleValueTests.swift index b33fffc..f3af622 100644 --- a/Tests/Mapper/ConvertibleValueTests.swift +++ b/Tests/Mapper/ConvertibleValueTests.swift @@ -1,6 +1,12 @@ import Mapper import XCTest +private struct Foo: Convertible { + static func fromMap(value: AnyObject?) throws -> Foo { + return Foo() + } +} + final class ConvertibleValueTests: XCTestCase { func testCreatingURL() { struct Test: Mappable { @@ -121,4 +127,82 @@ final class ConvertibleValueTests: XCTestCase { let test = try! Test(map: Mapper(JSON: [:])) XCTAssertNil(test.URL) } + + func testDictionaryConvertible() { + struct Test: Mappable { + let dictionary: [String: Int] + + init(map: Mapper) throws { + try self.dictionary = map.from("foo") + } + } + + let test = Test.from(["foo": ["key": 1]])! + XCTAssertTrue(test.dictionary["key"] == 1) + } + + func testOptionalDictionaryConvertible() { + struct Test: Mappable { + let dictionary: [String: Int]? + + init(map: Mapper) throws { + self.dictionary = map.optionalFrom("foo") + } + } + + let test = Test.from(["foo": ["key": 1]])! + XCTAssertTrue(test.dictionary?["key"] == 1) + } + + func testDictionaryOfConvertibles() { + struct Test: Mappable { + let dictionary: [String: Foo] + + init(map: Mapper) throws { + try self.dictionary = map.from("foo") + } + } + + let test = Test.from(["foo": ["key": "value"]]) + XCTAssertTrue(test?.dictionary.count > 0) + } + + func testOptionalDictionaryConvertibleNil() { + struct Test: Mappable { + let dictionary: [String: Int]? + + init(map: Mapper) throws { + self.dictionary = map.optionalFrom("foo") + } + } + + let test = Test.from(["foo": ["key": "not int"]])! + XCTAssertNil(test.dictionary) + } + + func testDictionaryConvertibleSingleInvalid() { + struct Test: Mappable { + let dictionary: [String: Int] + + init(map: Mapper) throws { + try self.dictionary = map.from("foo") + } + } + + let test = Test.from(["foo": ["key": 1, "key2": "not int"]]) + XCTAssertNil(test) + } + + func testDictionaryButInvalidJSON() { + struct Test: Mappable { + let dictionary: [String: Int] + + init(map: Mapper) throws { + try self.dictionary = map.from("foo") + } + } + + let test = Test.from(["foo": "not a dictionary"]) + XCTAssertNil(test) + } } diff --git a/Tests/Mapper/ErrorTests.swift b/Tests/Mapper/ErrorTests.swift index e02c430..b415f0e 100644 --- a/Tests/Mapper/ErrorTests.swift +++ b/Tests/Mapper/ErrorTests.swift @@ -3,14 +3,18 @@ import XCTest final class ErrorTests: XCTestCase { func testTypeMismatch() { + struct Test: Mappable { + init(map: Mapper) throws {} + } + do { let map = Mapper(JSON: ["field": 1]) - let _: String = try map.from("field") + let _: Test = try map.from("field") XCTFail() } catch MapperError.TypeMismatchError(let field, let value, let type) { XCTAssert(field == "field") XCTAssert(value as? Int == 1) - XCTAssert(type == String.self) + XCTAssert(type == NSDictionary.self) } catch { XCTFail() } diff --git a/Tests/Mapper/NormalValueTests.swift b/Tests/Mapper/NormalValueTests.swift index 00854c1..c22eab9 100644 --- a/Tests/Mapper/NormalValueTests.swift +++ b/Tests/Mapper/NormalValueTests.swift @@ -14,6 +14,18 @@ final class NormalValueTests: XCTestCase { XCTAssertTrue(test.string == "Hello") } + func testMappingTimeInterval() { + struct Test: Mappable { + let string: NSTimeInterval + init(map: Mapper) throws { + try self.string = map.from("time") + } + } + + let test = try! Test(map: Mapper(JSON: ["time": 123])) + XCTAssertTrue(test.string == 123) + } + func testMappingMissingKey() { struct Test: Mappable { let string: String @@ -52,7 +64,7 @@ final class NormalValueTests: XCTestCase { func testEmptyStringJSON() { struct Test: Mappable { - let JSON: AnyObject + let JSON: NSDictionary init(map: Mapper) throws { try self.JSON = map.from("") } @@ -86,4 +98,16 @@ final class NormalValueTests: XCTestCase { let test = try? Test(map: Mapper(JSON: ["strings": ["hi", 1]])) XCTAssertNil(test) } + + func testOptionalPropertyWithFrom() { + struct Test: Mappable { + let string: String? + init(map: Mapper) throws { + try self.string = map.from("string") + } + } + + let test = try? Test(map: Mapper(JSON: ["string": "hi"])) + XCTAssertEqual(test?.string, "hi") + } }