From 4ce74e2faf3ea960246f9b358359b7fc1fcf5137 Mon Sep 17 00:00:00 2001 From: Ben Wetherfield Date: Wed, 31 Jul 2019 14:15:11 -0700 Subject: [PATCH] Decoding choice elements that can hold empty structs (#120) ## Introduction In merging in #119, we fixed most but not quite all of #91! Decoding of _null_ choice elements (represented as enums with empty struct associated values on the Swift side) still results in errors. This PR adds a test to demonstrate and fixes this issue by wrapping each `NullBox` inside of a `SingleKeyedBox` at the `merge` phase (called by `transformToBoxTree`). ## Motivation One of the main lessons from #119 was that we have to wrap choice elements in the decoding phase to hold onto their keys. The keys are needed for directing us to the correct branch of the do-catch pyramid used for decoding. ```swift private enum Entry: Equatable { case run(Run) case properties(Properties) case br(Break) } extension Entry: Decodable { private enum CodingKeys: String, XMLChoiceCodingKey { case run, properties, br } public init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) do { self = .run(try container.decode(Run.self, forKey: .run)) } catch { do { self = .properties(try container.decode(Properties.self, forKey: .properties)) } catch { self = .br(try container.decode(Break.self, forKey: .br)) } } } } ``` where one of the associated values could be an empty struct (represented by null): ```swift private struct Break: Decodable {} ``` Although we _can_ throw out keys for non-choice null elements, a mechanism is needed for holding onto the keys while transforming from the `XMLCoderElement` tree to the `boxTree`. Only later will we know if the key is needed (if this wrapped element is transformed to a `ChoiceBox`); if not, we will be able to throw out the key. ## Proposed solution The Public API is unchanged. On the implementation side, we catch `NullBox` values in `merge` and wrap them in `SingleKeyedBox` instances. ## Detailed Design In `merge`, we wrap each `NullBox` in a `SingleKeyedBox` with the appropriate key bundled in. An `XMLChoiceDecodingContainer` can be constructed from the `SingleKeyedBox` by converting it to a `ChoiceBox` (just transferring over the contents) - as normal. In `XMLKeyedDecodingContainer`, when preparing the `elements` for concrete decoding, we unwrap all `SingleKeyedBox` values that may be contained therein, as any choice elements contained would have already been transformed to a `ChoiceBox` by this point in decoding: any stray `SingleKeyedBox` wrappers can thus be thrown out. ## Source compatibility This is purely an additive change. --- .../XMLCoder/Auxiliaries/KeyedStorage.swift | 2 +- .../Decoder/XMLKeyedDecodingContainer.swift | 20 ++++++++- Tests/XMLCoderTests/NestedChoiceTests.swift | 44 +++++++++++++++++++ 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/Sources/XMLCoder/Auxiliaries/KeyedStorage.swift b/Sources/XMLCoder/Auxiliaries/KeyedStorage.swift index 49a82d0d..740455fa 100644 --- a/Sources/XMLCoder/Auxiliaries/KeyedStorage.swift +++ b/Sources/XMLCoder/Auxiliaries/KeyedStorage.swift @@ -83,7 +83,7 @@ extension KeyedStorage where Key == String, Value == Box { } else if let value = element.value { result.append(StringBox(value), at: element.key) } else { - result.append(NullBox(), at: element.key) + result.append(SingleKeyedBox(key: element.key, element: NullBox()), at: element.key) } return result diff --git a/Sources/XMLCoder/Decoder/XMLKeyedDecodingContainer.swift b/Sources/XMLCoder/Decoder/XMLKeyedDecodingContainer.swift index cbf42eb2..e236db91 100644 --- a/Sources/XMLCoder/Decoder/XMLKeyedDecodingContainer.swift +++ b/Sources/XMLCoder/Decoder/XMLKeyedDecodingContainer.swift @@ -79,6 +79,10 @@ struct XMLKeyedDecodingContainer: KeyedDecodingContainerProtocol { let box = elements.first ?? attributes.first + if let singleKeyed = box as? SingleKeyedBox { + return singleKeyed.element.isNull + } + return box?.isNull ?? true } @@ -232,14 +236,26 @@ extension XMLKeyedDecodingContainer { let keyString = key.stringValue.isEmpty ? "value" : key.stringValue let value = keyedBox.elements[keyString] if !value.isEmpty { - return value + return value.map { + if let singleKeyed = $0 as? SingleKeyedBox { + return singleKeyed.element + } else { + return $0 + } + } } else if let value = keyedBox.value { return [value] } else { return [] } } else { - return keyedBox.elements[key.stringValue] + return keyedBox.elements[key.stringValue].map { + if let singleKeyed = $0 as? SingleKeyedBox { + return singleKeyed.element + } else { + return $0 + } + } } } diff --git a/Tests/XMLCoderTests/NestedChoiceTests.swift b/Tests/XMLCoderTests/NestedChoiceTests.swift index 4810c454..44344c49 100644 --- a/Tests/XMLCoderTests/NestedChoiceTests.swift +++ b/Tests/XMLCoderTests/NestedChoiceTests.swift @@ -227,6 +227,50 @@ class NestedChoiceTests: XCTestCase { XCTAssertEqual(result, expected) } + func testNestedEnumsWithEmptyStruct() throws { + let xml = """ + +

+

+ + 1518 + I am answering it again. + + + 431 + A Word About Wake Times + +

+

+ + 1519 + I am answering it again. + +
+

+ + """ + let result = try XMLDecoder().decode(Container.self, from: xml.data(using: .utf8)!) + let expected = Container( + paragraphs: [ + Paragraph( + entries: [ + .br(Break()), + .run(Run(id: 1518, text: "I am answering it again.")), + .properties(Properties(id: 431, title: "A Word About Wake Times")), + ] + ), + Paragraph( + entries: [ + .run(Run(id: 1519, text: "I am answering it again.")), + .br(Break()), + ] + ), + ] + ) + XCTAssertEqual(result, expected) + } + func testNestedEnumsRoundTrip() throws { let original = Container( paragraphs: [