tristanhimmelman/AlamofireObjectMapper

ImmutableMappable fails whole array response if one object errors on deserialization

beltex opened this issue · 0 comments

First off, thanks for the great project!

This issue could just as well be posted to the ObjectMapper repo, but I thought I'd post it here nonetheless, since it affects this project as well.

Scenario

Given an endpoint that returns say a list of photo objects that are all ImmutableMappable, if any one of them fails to deserialize, the whole response is treated as a failure and an error is thrown. This is due to ImmutableMappable.mapArray(). While it does a compactMap like its Mappable counterpart, it throws in addition. Thus, it's behaviour differs, in that with Mappable, those particular objects would simply be evicted from the response array.

Question

Is there a reason for the existing approach or is it a bug?

Proposed Solution

Make the behaviour match Mappable by catching errors and simply returning nil (which would get filtered out by the compactMap). I believe this is the behaviour most users would want/expect. If all the objects fail to deserialize, you'll simply have an empty array response.

In effect, the change would be something like this (in ObjectMapper - would need to change the other variants as well):

diff --git a/Sources/ImmutableMappable.swift b/Sources/ImmutableMappable.swift
index 27b6315..4988710 100644
--- a/Sources/ImmutableMappable.swift
+++ b/Sources/ImmutableMappable.swift
@@ -215,7 +215,7 @@ public extension Mapper where N: ImmutableMappable {

        public func mapArray(JSONArray: [[String: Any]]) throws -> [N] {
                #if swift(>=4.1)
-               return try JSONArray.compactMap(mapOrFail)
+               return JSONArray.compactMap { try? mapOrFail(JSON: $0) }
                #else
                return try JSONArray.flatMap(mapOrFail)
                #endif