Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/fails on empty data for decodable #1508

Merged
merged 9 commits into from Dec 29, 2017
1 change: 1 addition & 0 deletions Changelog.md
@@ -1,6 +1,7 @@
# Next
### Added
- **Breaking Change** Added a `.requestCustomJSONEncodable` case to `Task`. [#1443](https://github.com/Moya/Moya/pull/1443) by [@evgeny-sureev](https://github.com/evgeny-sureev).
- **Breaking Change** Added `failsOnEmptyData` boolean support for the `Decodable` map functions. [#1508](https://github.com/Moya/Moya/pull/1508) by [@jeroenbb94](https://github.com/Jeroenbb94).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this isn't the main point of this PR, but is it really a breaking change? There is a default argument for this parameter, this parameter is a last one, and if you don't provide an option you will get the same behavior as in previous versions. Or am I missing something? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @sunshinejr that was me 😅
#1508 (comment)

Copy link
Member

@SD10 SD10 Dec 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My definition of breaking is anything that will prevent the project from compiling. Changes in variable names, type names, function signatures, adding/removing cases from an enum, or changes to a more restrictive form of access control.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! You are absolutely correct @SD10, I just don't see any issue that could break it, do you have anything particular in mind?

Copy link
Member

@SD10 SD10 Dec 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing in particular, I just go by those general rules 😅 Someone could be relying on the function signature of Response.map. Using it as an argument of a higher order function maybe?


### Changed
- **Breaking Change** Updated minimum version of `ReactiveSwift` to 3.0.
Expand Down
20 changes: 16 additions & 4 deletions Sources/Moya/Response.swift
Expand Up @@ -121,7 +121,7 @@ public extension Response {
///
/// - parameter atKeyPath: Optional key path at which to parse object.
/// - parameter using: A `JSONDecoder` instance which is used to decode data to an object.
func map<D: Decodable>(_ type: D.Type, atKeyPath keyPath: String? = nil, using decoder: JSONDecoder = JSONDecoder()) throws -> D {
func map<D: Decodable>(_ type: D.Type, atKeyPath keyPath: String? = nil, using decoder: JSONDecoder = JSONDecoder(), failsOnEmptyData: Bool = true) throws -> D {
let serializeToData: (Any) throws -> Data? = { (jsonObject) in
guard JSONSerialization.isValidJSONObject(jsonObject) else {
return nil
Expand All @@ -133,9 +133,14 @@ public extension Response {
}
}
let jsonData: Data
if let keyPath = keyPath {
guard let jsonObject = (try mapJSON() as? NSDictionary)?.value(forKeyPath: keyPath) else {
throw MoyaError.jsonMapping(self)
keyPathCheck: if let keyPath = keyPath {
guard let jsonObject = (try mapJSON(failsOnEmptyData: failsOnEmptyData) as? NSDictionary)?.value(forKeyPath: keyPath) else {
if failsOnEmptyData {
throw MoyaError.jsonMapping(self)
} else {
jsonData = data
break keyPathCheck
}
}

if let data = try serializeToData(jsonObject) {
Expand All @@ -158,6 +163,13 @@ public extension Response {
jsonData = data
}
do {
if jsonData.count < 1 && !failsOnEmptyData {
if let emptyJSONObjectData = "{}".data(using: .utf8), let emptyDecodableValue = try? decoder.decode(D.self, from: emptyJSONObjectData) {
return emptyDecodableValue
} else if let emptyJSONArrayData = "[{}]".data(using: .utf8), let emptyDecodableValue = try? decoder.decode(D.self, from: emptyJSONArrayData) {
return emptyDecodableValue
}
}
return try decoder.decode(D.self, from: jsonData)
} catch let error {
throw MoyaError.objectMapping(error, self)
Expand Down
4 changes: 2 additions & 2 deletions Sources/ReactiveMoya/SignalProducer+Response.swift
Expand Up @@ -54,9 +54,9 @@ extension SignalProducerProtocol where Value == Response, Error == MoyaError {
}

/// Maps received data at key path into a Decodable object. If the conversion fails, the signal errors.
public func map<D: Decodable>(_ type: D.Type, atKeyPath keyPath: String? = nil, using decoder: JSONDecoder = JSONDecoder()) -> SignalProducer<D, MoyaError> {
public func map<D: Decodable>(_ type: D.Type, atKeyPath keyPath: String? = nil, using decoder: JSONDecoder = JSONDecoder(), failsOnEmptyData: Bool = true) -> SignalProducer<D, MoyaError> {
return producer.flatMap(.latest) { response -> SignalProducer<D, MoyaError> in
return unwrapThrowable { try response.map(type, atKeyPath: keyPath, using: decoder) }
return unwrapThrowable { try response.map(type, atKeyPath: keyPath, using: decoder, failsOnEmptyData: failsOnEmptyData) }
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/RxMoya/Observable+Response.swift
Expand Up @@ -54,9 +54,9 @@ extension ObservableType where E == Response {
}

/// Maps received data at key path into a Decodable object. If the conversion fails, the signal errors.
public func map<D: Decodable>(_ type: D.Type, atKeyPath keyPath: String? = nil, using decoder: JSONDecoder = JSONDecoder()) -> Observable<D> {
public func map<D: Decodable>(_ type: D.Type, atKeyPath keyPath: String? = nil, using decoder: JSONDecoder = JSONDecoder(), failsOnEmptyData: Bool = true) -> Observable<D> {
return flatMap { response -> Observable<D> in
return Observable.just(try response.map(type, atKeyPath: keyPath, using: decoder))
return Observable.just(try response.map(type, atKeyPath: keyPath, using: decoder, failsOnEmptyData: failsOnEmptyData))
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions Sources/RxMoya/Single+Response.swift
Expand Up @@ -54,9 +54,9 @@ extension PrimitiveSequence where TraitType == SingleTrait, ElementType == Respo
}

/// Maps received data at key path into a Decodable object. If the conversion fails, the signal errors.
public func map<D: Decodable>(_ type: D.Type, atKeyPath keyPath: String? = nil, using decoder: JSONDecoder = JSONDecoder()) -> Single<D> {
public func map<D: Decodable>(_ type: D.Type, atKeyPath keyPath: String? = nil, using decoder: JSONDecoder = JSONDecoder(), failsOnEmptyData: Bool = true) -> Single<D> {
return flatMap { response -> Single<D> in
return Single.just(try response.map(type, atKeyPath: keyPath, using: decoder))
return Single.just(try response.map(type, atKeyPath: keyPath, using: decoder, failsOnEmptyData: failsOnEmptyData))
}
}
}
4 changes: 4 additions & 0 deletions Tests/.swiftlint.yml
Expand Up @@ -6,6 +6,10 @@ disabled_rules:
- force_try
- function_body_length
- cyclomatic_complexity
- type_body_length

opt_in_rules:
- empty_count

file_length:
warning: 1000
49 changes: 49 additions & 0 deletions Tests/Observable+MoyaSpec.swift
Expand Up @@ -309,6 +309,30 @@ class ObservableMoyaSpec: QuickSpec {
expect(receivedObjects?.count) == 3
expect(receivedObjects?.map { $0.title }) == ["Hello, Moya!", "Hello, Moya!", "Hello, Moya!"]
}
it("maps empty data to a decodable object with optional properties") {
let observable = Response(statusCode: 200, data: Data()).asObservable()

var receivedObjects: OptionalIssue?
_ = observable.map(OptionalIssue.self, using: decoder, failsOnEmptyData: false).subscribe(onNext: { object in
receivedObjects = object
})
expect(receivedObjects).notTo(beNil())
expect(receivedObjects?.title).to(beNil())
expect(receivedObjects?.createdAt).to(beNil())
}

it("maps empty data to a decodable array with optional properties") {
let observable = Response(statusCode: 200, data: Data()).asObservable()

var receivedObjects: [OptionalIssue]?
_ = observable.map([OptionalIssue].self, using: decoder, failsOnEmptyData: false).subscribe(onNext: { object in
receivedObjects = object
})
expect(receivedObjects).notTo(beNil())
expect(receivedObjects?.count) == 1
expect(receivedObjects?.first?.title).to(beNil())
expect(receivedObjects?.first?.createdAt).to(beNil())
}

context("when using key path mapping") {
it("maps data representing a json to a decodable object") {
Expand Down Expand Up @@ -344,6 +368,31 @@ class ObservableMoyaSpec: QuickSpec {
expect(receivedObjects?.first?.createdAt) == formatter.date(from: "1995-01-14T12:34:56")!
}

it("maps empty data to a decodable object with optional properties") {
let observable = Response(statusCode: 200, data: Data()).asObservable()

var receivedObjects: OptionalIssue?
_ = observable.map(OptionalIssue.self, atKeyPath: "issue", using: decoder, failsOnEmptyData: false).subscribe(onNext: { object in
receivedObjects = object
})
expect(receivedObjects).notTo(beNil())
expect(receivedObjects?.title).to(beNil())
expect(receivedObjects?.createdAt).to(beNil())
}

it("maps empty data to a decodable array with optional properties") {
let observable = Response(statusCode: 200, data: Data()).asObservable()

var receivedObjects: [OptionalIssue]?
_ = observable.map([OptionalIssue].self, atKeyPath: "issue", using: decoder, failsOnEmptyData: false).subscribe(onNext: { object in
receivedObjects = object
})
expect(receivedObjects).notTo(beNil())
expect(receivedObjects?.count) == 1
expect(receivedObjects?.first?.title).to(beNil())
expect(receivedObjects?.first?.createdAt).to(beNil())
}

it("map Int data to an Int value") {
let json: [String: Any] = ["count": 1]
guard let data = try? JSONSerialization.data(withJSONObject: json, options: .prettyPrinted) else {
Expand Down
50 changes: 50 additions & 0 deletions Tests/SignalProducer+MoyaSpec.swift
Expand Up @@ -295,6 +295,31 @@ class SignalProducerMoyaSpec: QuickSpec {
expect(receivedObjects?.map { $0.title }) == ["Hello, Moya!", "Hello, Moya!", "Hello, Moya!"]
}

it("maps empty data to a decodable object with optional properties") {
let signal = signalSendingData(Data())

var receivedObjects: OptionalIssue?
_ = signal.map(OptionalIssue.self, using: decoder, failsOnEmptyData: false).startWithResult { result in
receivedObjects = result.value
}
expect(receivedObjects).notTo(beNil())
expect(receivedObjects?.title).to(beNil())
expect(receivedObjects?.createdAt).to(beNil())
}

it("maps empty data to a decodable array with optional properties") {
let signal = signalSendingData(Data())

var receivedObjects: [OptionalIssue]?
_ = signal.map([OptionalIssue].self, using: decoder, failsOnEmptyData: false).startWithResult { result in
receivedObjects = result.value
}
expect(receivedObjects).notTo(beNil())
expect(receivedObjects?.count) == 1
expect(receivedObjects?.first?.title).to(beNil())
expect(receivedObjects?.first?.createdAt).to(beNil())
}

context("when using key path mapping") {
it("maps data representing a json to a decodable object") {
let json: [String: Any] = ["issue": json] // nested json
Expand Down Expand Up @@ -329,6 +354,31 @@ class SignalProducerMoyaSpec: QuickSpec {
expect(receivedObjects?.first?.createdAt) == formatter.date(from: "1995-01-14T12:34:56")!
}

it("maps empty data to a decodable object with optional properties") {
let signal = signalSendingData(Data())

var receivedObjects: OptionalIssue?
_ = signal.map(OptionalIssue.self, atKeyPath: "issue", using: decoder, failsOnEmptyData: false).startWithResult { result in
receivedObjects = result.value
}
expect(receivedObjects).notTo(beNil())
expect(receivedObjects?.title).to(beNil())
expect(receivedObjects?.createdAt).to(beNil())
}

it("maps empty data to a decodable array with optional properties") {
let signal = signalSendingData(Data())

var receivedObjects: [OptionalIssue]?
_ = signal.map([OptionalIssue].self, atKeyPath: "issue", using: decoder, failsOnEmptyData: false).startWithResult { result in
receivedObjects = result.value
}
expect(receivedObjects).notTo(beNil())
expect(receivedObjects?.count) == 1
expect(receivedObjects?.first?.title).to(beNil())
expect(receivedObjects?.first?.createdAt).to(beNil())
}

it("map Int data to an Int value") {
let json: [String: Any] = ["count": 1]
guard let data = try? JSONSerialization.data(withJSONObject: json, options: .prettyPrinted) else {
Expand Down
50 changes: 50 additions & 0 deletions Tests/Single+MoyaSpec.swift
Expand Up @@ -305,6 +305,31 @@ class SingleMoyaSpec: QuickSpec {
expect(receivedObjects?.map { $0.title }) == ["Hello, Moya!", "Hello, Moya!", "Hello, Moya!"]
}

it("maps empty data to a decodable object with optional properties") {
let single = Response(statusCode: 200, data: Data()).asSingle()

var receivedObjects: OptionalIssue?
_ = single.map(OptionalIssue.self, using: decoder, failsOnEmptyData: false).subscribe(onSuccess: { object in
receivedObjects = object
})
expect(receivedObjects).notTo(beNil())
expect(receivedObjects?.title).to(beNil())
expect(receivedObjects?.createdAt).to(beNil())
}

it("maps empty data to a decodable array with optional properties") {
let single = Response(statusCode: 200, data: Data()).asSingle()

var receivedObjects: [OptionalIssue]?
_ = single.map([OptionalIssue].self, using: decoder, failsOnEmptyData: false).subscribe(onSuccess: { object in
receivedObjects = object
})
expect(receivedObjects).notTo(beNil())
expect(receivedObjects?.count) == 1
expect(receivedObjects?.first?.title).to(beNil())
expect(receivedObjects?.first?.createdAt).to(beNil())
}

context("when using key path mapping") {
it("maps data representing a json to a decodable object") {
let json: [String: Any] = ["issue": json] // nested json
Expand Down Expand Up @@ -339,6 +364,31 @@ class SingleMoyaSpec: QuickSpec {
expect(receivedObjects?.first?.createdAt) == formatter.date(from: "1995-01-14T12:34:56")!
}

it("maps empty data to a decodable object with optional properties") {
let single = Response(statusCode: 200, data: Data()).asSingle()

var receivedObjects: OptionalIssue?
_ = single.map(OptionalIssue.self, atKeyPath: "issue", using: decoder, failsOnEmptyData: false).subscribe(onSuccess: { object in
receivedObjects = object
})
expect(receivedObjects).notTo(beNil())
expect(receivedObjects?.title).to(beNil())
expect(receivedObjects?.createdAt).to(beNil())
}

it("maps empty data to a decodable array with optional properties") {
let single = Response(statusCode: 200, data: Data()).asSingle()

var receivedObjects: [OptionalIssue]?
_ = single.map([OptionalIssue].self, atKeyPath: "issue", using: decoder, failsOnEmptyData: false).subscribe(onSuccess: { object in
receivedObjects = object
})
expect(receivedObjects).notTo(beNil())
expect(receivedObjects?.count) == 1
expect(receivedObjects?.first?.title).to(beNil())
expect(receivedObjects?.first?.createdAt).to(beNil())
}

it("map Int data to an Int value") {
let json: [String: Any] = ["count": 1]
guard let data = try? JSONSerialization.data(withJSONObject: json, options: .prettyPrinted) else {
Expand Down
9 changes: 7 additions & 2 deletions Tests/TestHelpers.swift
Expand Up @@ -51,8 +51,7 @@ extension GitHub: TargetType {
}

extension GitHub: Equatable {

static func ==(lhs: GitHub, rhs: GitHub) -> Bool {
static func == (lhs: GitHub, rhs: GitHub) -> Bool {
switch (lhs, rhs) {
case (.zen, .zen): return true
case let (.userProfile(username1), .userProfile(username2)): return username1 == username2
Expand Down Expand Up @@ -233,3 +232,9 @@ struct Issue: Codable {
case rating
}
}

// A fixture for testing optional Decodable mapping
struct OptionalIssue: Codable {
let title: String?
let createdAt: Date?
}
70 changes: 69 additions & 1 deletion docs/Examples/Response.md
Expand Up @@ -214,8 +214,9 @@ struct User: Decodable {
}
```

Moya allows us to easily get our `User` from the response with the `map<D: Decodable>(_: D.Type, atKeyPath: String?, using: JSONDecoder)` extension.
Moya allows us to easily get our `User` from the response with the `map<D: Decodable>(_: D.Type, atKeyPath: String?, using: JSONDecoder, failsOnEmptyData: Bool)` extension.
Both `atKeyPath` and `using` are optional, meaning in most cases you'll use `map(_:)`.
The `failsOnEmptyData` property (default: true), describes whether it should throw an error when data is empty or simply return `Decodable` initialized with nil (note: your object must allow optionals or you'll still get thrown an error).
A basic example would be:

```swift
Expand Down Expand Up @@ -337,3 +338,70 @@ provider.rx.request(.users)
}
}
```

The above assumes your backend always returns data and if it doesn't, throwns an error.
But if you don't want to receive an error, we can set `failsOnEmptyData` to false.

The data returned looks like this:

```json
{
"users": []
}
```

Our updated `User` type looks like this:

```swift
struct User: Decodable {
let id: String?
let firstName: String?
let lastName: String?
let updated: Date?
}
```

Our handling of the result now has to do slightly more:

```swift
provider.request(.user) { result in
switch result {
case let .success(moyaResponse):
do {
let filteredResponse = try moyaResponse.filterSuccessfulStatusCodes()
let decoder = JSONDecoder()
decoder.dateDecodingStrategy = .secondsSince1970
let users = try filteredResponse.map([User].self, atKeyPath: "users", using: decoder, failsOnEmptyData: false) // user is of type [User]
// Because the failsOnEmptyData is false and our user object allows optional, our array got initialized with an empty User object
// Do something with your users.
}
catch let error {
// Here we get either statusCode error or objectMapping error.
// TODO: handle the error == best. comment. ever.
}
case let .failure(error):
// TODO: handle the error == best. comment. ever.
}
}
```

In `RxSwift` this could look something like:

```swift
let decoder = JSONDecoder()
decoder.dateDecodingStrategy = .secondsSince1970
provider.rx.request(.users)
.filterSuccessfulStatusCodes()
.map([User].self, atKeyPath: "users", using: decoder, failsOnEmptyData: false)
.subscribe { event in
switch event {
case .success(let users):
// Notice that now we do not get a Response object anymore but rather an array of User objects
// Because the failsOnEmptyData is false and our user object allows optional, our array got initialized with an empty User object
// do something with the user
case .error(let error):
// handle the error, which can be an underlying error, a status code error, or an object mapping error
}
}
}
```