Skip to content

Commit

Permalink
LossyDictionary: added failing test and fix for key conversion issue
Browse files Browse the repository at this point in the history
The property wrapper `LossyDictionary` incorrectly decodes keys with `KeyDecodingStrategy.convertFromSnakeCase`.
The test simply encodes and decodes a sample data, to illustrate that the conversion isn't idempotent.

Keys were being converted to camel case, which is inconsistent from the default behavior that `container.decode([String: Value].self, forKey: key)` would have.
The reason for this is some implementation detail in `Foundation` that ignores the `keyDecodingStrategy` when decoding "raw" dictionaries versus property names.

To deal with this, this decodes the strings first as "raw", and then matches them to the corresponding converted key.
  • Loading branch information
NachoSoto committed Apr 28, 2022
1 parent 57d55d7 commit f5a86a7
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
20 changes: 18 additions & 2 deletions Sources/BetterCodable/LossyDictionary.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,12 @@ extension LossyDictionary: Decodable where Key: Decodable, Value: Decodable {
var elements: [Key: Value] = [:]
if Key.self == String.self {
let container = try decoder.container(keyedBy: DictionaryCodingKey.self)
let keys = try Self.extractKeys(from: decoder, container: container)

for key in container.allKeys {
for (key, stringKey) in keys {
do {
let value = try container.decode(LossyDecodableValue<Value>.self, forKey: key).value
elements[key.stringValue as! Key] = value
elements[stringKey as! Key] = value
} catch {
_ = try? container.decode(AnyDecodableValue.self, forKey: key)
}
Expand Down Expand Up @@ -81,6 +82,21 @@ extension LossyDictionary: Decodable where Key: Decodable, Value: Decodable {

self.wrappedValue = elements
}

private static func extractKeys(
from decoder: Decoder,
container: KeyedDecodingContainer<DictionaryCodingKey>
) throws -> [(DictionaryCodingKey, String)] {
// Decode a dictionary ignoring the values to decode the original keys
// without using the `JSONDecoder.KeyDecodingStrategy`.
let keys = try decoder.singleValueContainer().decode([String: AnyDecodableValue].self).keys

return zip(
container.allKeys.sorted(by: { $0.stringValue < $1.stringValue }),
keys.sorted()
)
.map { ($0, $1) }
}
}

extension LossyDictionary: Encodable where Key: Encodable, Value: Encodable {
Expand Down
23 changes: 23 additions & 0 deletions Tests/BetterCodableTests/LossyDictionaryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,27 @@ class LossyDictionaryTests: XCTestCase {
XCTAssertEqual(fixture.stringToInt, ["one": 1, "two": 2, "three": 3])
XCTAssertEqual(fixture.intToString, [1: "one", 2: "two", 3: "three"])
}

func testEncodingLosslessDictionaryRetainsKeys() throws {
let decoder = JSONDecoder()
decoder.keyDecodingStrategy = .convertFromSnakeCase

let encoder = JSONEncoder()
encoder.keyEncodingStrategy = .convertToSnakeCase

let values: [String: Int] = [
"snake_case.with.dots_99.and_numbers": 1,
"dots.and_2.00.1_numbers": 2,
"key.1": 3
]

let fixture = Fixture(
stringToInt: values,
intToString: [:]
)

let reencodedFixture = try decoder.decode(Fixture.self, from: encoder.encode(fixture))

XCTAssertEqual(reencodedFixture, fixture)
}
}

0 comments on commit f5a86a7

Please # to comment.