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

[SR-5953] Decodable: Allow "Lossy" Array Decodes #4414

Open
swift-ci opened this issue Sep 21, 2017 · 9 comments
Open

[SR-5953] Decodable: Allow "Lossy" Array Decodes #4414

swift-ci opened this issue Sep 21, 2017 · 9 comments

Comments

@swift-ci
Copy link
Contributor

Previous ID SR-5953
Radar rdar://problem/37316307
Original Reporter cocoahero (JIRA User)
Type Improvement

Attachment: Download

Additional Detail from JIRA
Votes 26
Component/s Foundation
Labels Improvement
Assignee None
Priority Medium

md5: 8e5158944f566da394566f3054ff764a

Issue Description:

In the current implementation of Decodable, it is impossible to attempt to decode an array of Decodable's and allow individual items to fail.

For example:

enum Season: String, Decodable {
    case winter
    case spring
    case summer
    case autumn
}

let json = """
["winter", "spring", "summer", "fall"]
""".data(using: .utf8)!

let seasons = try! JSONDecoder().decode([Season].self, from: json)

This will raise an error of the following:

Swift.DecodingError.dataCorrupted(Swift.DecodingError.Context(codingPath: [Foundation.(_JSONKey in _12768CA107A31EF2DCE034FD75B541C9)(stringValue: "Index 3", intValue: Optional(3))], debugDescription: "Cannot initialize Season from invalid String value fall", underlyingError: nil))

It may be considered desirable in certain use cases that this decode succeeds, but omits individual items that fail. As such, the desired output value would be:

[.winter, .spring. summer]

One may attempt to circumvent this by using an outer, wrapper type combined with a customized initializer that ignores inner decode errors, such as the following:

struct Climate: Decodable {
    var seasons: [Season]

    enum CodingKeys: String, CodingKey {
        case seasons
    }

    init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        var seasonsContainer = try container.nestedUnkeyedContainer(forKey: .seasons)

        var seasons = [Season]()
        while !seasonsContainer.isAtEnd {
            if let season = try? seasonsContainer.decode(Season.self) {
                seasons.append(season)
            }
        }
        self.seasons = seasons
    }
}

let json = """
{ "seasons": ["winter", "spring", "summer", "fall"] }
""".data(using: .utf8)!

let climate = try! JSONDecoder().decode(Climate.self, from: json)

However, the above code will result in an infinite loop due to the implementation of UnkeyedDecodingContainer.currentIndex. This is because the currentIndex is not incremented unless a decode succeeds.

@belkadan
Copy link

cc @itaiferber

@itaiferber
Copy link
Contributor

The reason a failed decode does not skip a value is to allow repeated attempts to decode as different types, e.g.

// The next element can be either a number or a string. Try both.
do {
    let number = try container.decode(Double.self)
    // ... do something with number
    return
} catch { ... }

do {
    // The value wasn't a number; try a string.
    let string = try container.decode(String.self)
    // ... do something with string
    return
} catch { ... }

It may be possible to add API to allow skipping elements, but at this point, an addition to the protocols will likely be a breaking change.

@itaiferber
Copy link
Contributor

cocoahero (JIRA User) FWIW, you can always break out of the loop early by detecting that season is nil or that the index has not incremented.

@swift-ci
Copy link
Contributor Author

Comment by Jonathan Baker (JIRA)

@itaiferber I understand that I can prevent the infinite loop by breaking out, but that would prevent decoding of further indexes down the array, essentially creating a prefix slice of my intended results.

This bug report isn't so much about the behavior of UnkeyedDecodingContainer.currentIndex, but rather the inability to effectively skip failed items in an Array decode.

@krilnon
Copy link
Member

krilnon commented Sep 22, 2017

You can operate around values you don't know how to decode with an empty codable type:

struct AnyCodable: Codable {}

while !seasonsContainer.isAtEnd {   
    if let season = try? seasonsContainer.decode(Season.self) {
        seasons.append(season)
    } else {
        let skipped = try? seasonsContainer.decode(AnyCodable.self)
        print("skipping one \(skipped)")
    }
}

Throwing away data like this comes with its own set of tradeoffs, however.

@itaiferber
Copy link
Contributor

@krilnon Good point! That seems like a reasonable way around this at the moment.

@swift-ci
Copy link
Contributor Author

swift-ci commented Feb 7, 2018

Comment by Basem Emara (JIRA)

Thank you Swift team for the native JSON support and @itaiferber for the lossy JSON workaround. I do understand enforcing data integrity, but the web isn't so perfect or compile-safe. This is the very nature of JSON, aka [String: Any]. Please consider adding support for failable decodable such as: JSONDecoder().decode(Climate.self, from: json, options: [.skipFailable])

The irony is forcing us to write code like below is less safe:

enum Season: String, Decodable {
    case winter
    case spring
    case summer
    case autumn
}

struct Climate: Decodable {
    var seasons: [Season]
    
    
    enum CodingKeys: String, CodingKey {
        case seasons
    }
    
    init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        self.seasons = try container.decode(FailableCodableArray<Season>.self, forKey: .seasons).elements
    }
}

struct FailableCodableArray<Element: Decodable>: Decodable {
    // https://github.com/phynet/Lossy-array-decode-swift4
    private struct DummyCodable: Codable {}
    
    private struct FailableDecodable<Base: Decodable>: Decodable {
        let base: Base?
        
        init(from decoder: Decoder) throws {
            let container = try decoder.singleValueContainer()
            self.base = try? container.decode(Base.self)
        }
    }
    
    private(set) var elements: [Element]
    
    init(from decoder: Decoder) throws {
        var container = try decoder.unkeyedContainer()
        var elements = [Element]()
        
        if let count = container.count {
            elements.reserveCapacity(count)
        }
        
        while !container.isAtEnd {
            guard let element = try container.decode(FailableDecodable<Element>.self).base else {
                _ = try? container.decode(DummyCodable.self)
                continue
            }
            
            elements.append(element)
        }
        
        self.elements = elements
    }
}

let json = """
{ "seasons": ["winter", "spring", "summer", "fall"] }
""".data(using: .utf8)!

let climate = try! JSONDecoder().decode(Climate.self, from: json)

print(climate)

@itaiferber
Copy link
Contributor

@swift-ci Create

@swift-ci
Copy link
Contributor Author

Comment by Haoxin Li (JIRA)

I faced the same problem with infinite loop and this is my solution:

extension KeyedDecodingContainer {
    
    private struct EmptyDecodable: Decodable {}
    
    func safelyDecodeArray<T: Decodable>(of type: T.Type, forKey key: KeyedDecodingContainer.Key) -> [T] {
        guard var container = try? nestedUnkeyedContainer(forKey: key) else {
            return []
        }
        var elements = [T]()
        elements.reserveCapacity(container.count ?? 0)
        while !container.isAtEnd {
            do {
                elements.append(try container.decode(T.self))
            } catch {
                if let decodingError = error as? DecodingError {
                    Logger.error("\(#function): skipping one element: \(decodingError)")
                } else {
                    Logger.error("\(#function): skipping one element: \(error)")
                }
                _ = try? container.decode(EmptyDecodable.self)
            }
        }
        return elements
    }
}

There are still a lot can be done regarding failure handling.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants