Mixed choice/non-choice encoding #154

bwetherfield · 2019-11-25T00:30:19Z

Overview

Fixes bug encountered when encoding structs that hold a mixture of choice-element and non-choice-element (or multiple choice-element) properties.

Example

Given a structure that stores both a choice and non-choice property,

private struct IntOrStringAndDouble: Equatable {
    let intOrString: IntOrString
    let decimal: Double
}

the natural encoding approach (now available) is

extension IntOrStringAndDouble: Encodable {
    enum CodingKeys: String, CodingKey {
        case decimal
    }

    func encode(to encoder: Encoder) {
        try intOrString.encode(to: encoder)
        var container = encoder.container(keyedBy: CodingKeys.self)
        try container.encode(decimal, forKey: .decimal)
    }
}

The following encode implementation also works

extension IntOrStringAndDouble: Encodable {
    enum CodingKeys: String, CodingKey {
        case decimal
    }

    func encode(to encoder: Encoder) {
        var singleValueContainer = encoder.singleValueContainer()
        try singleValueContainer.encode(intOrString)
        var container = encoder.container(keyedBy: CodingKeys.self)
        try container.encode(decimal, forKey: .decimal)
    }
}

IntOrString as defined in #119

enum IntOrString: Equatable {
    case int(Int)
    case string(String)
}

extension IntOrString: Encodable {
    enum CodingKeys: String, CodingKey {
        case int
        case string
    }

    func encode(to encoder: Encoder) throws {
        var container = encoder.container(keyedBy: CodingKeys.self)
        switch self {
        case let .int(value):
            try container.encode(value, forKey: .int)
        case let .string(value):
            try container.encode(value, forKey: .string)
        }
    }
}

extension IntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `IntOrString` is a choice element

Implementation Details

In cases where choice and non-choice elements (or multiple choice elements) co-exist in a keyed container, we merge them into a single XMLKeyedEncodingContainer (wrapping a SharedBox<KeyedBox>). Arrays of choice elements (using XMLUnkeyedEncodingContainer under the hood) are encoded the same way as before, as we do not hit the merging cases. For the array case, we still need the XMLChoiceEncodingContainer structure.

Source Compatibility

This is an additive change.

MaxDesiatov · 2019-11-26T11:10:28Z

Sources/XMLCoder/Encoder/XMLEncoderImplementation.swift

            // We haven't yet pushed a container at this level; do so here.
-            topContainer = storage.pushChoiceContainer()
+            let container = XMLChoiceEncodingContainer<Key>(


likewise here

MaxDesiatov · 2019-11-26T11:10:41Z

Sources/XMLCoder/Encoder/XMLEncoderImplementation.swift

            // We haven't yet pushed a container at this level; do so here.
-            topContainer = storage.pushKeyedContainer()
+            let container = XMLKeyedEncodingContainer<Key>(


the comment above seems to be outdated now

MaxDesiatov · 2019-11-26T11:12:30Z

Sources/XMLCoder/Encoder/XMLEncoderImplementation.swift

-                    Attempt to push new (single element) keyed encoding container when already \
-                    previously encoded at this path.
+                    Attempt to push new keyed encoding container when already previously encoded \
+                    at this path.
                    """
                )


Overall this switch seems to be a duplicate of switch in public func keyedContainer modified above, could it be generalized somehow?

Ah, good point!

Sources/XMLCoder/Encoder/XMLEncoderImplementation.swift

MaxDesiatov

Amazing, many thanks!

Mirrors #154 Fixes bugs 1) Decoding multiple choice elements in the same Keyed Container. 2) Decoding choice elements after decoding regular keyed elements in the same container. Case 1 refers to decoding implementations of the form: ```swift init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) otherValue = try container.decode(String.self, forKey: .otherValue) intOrString = try IntOrString(from: decoder) } ``` where `IntOrString` is a choice coding element `IntOrString` defined as follows: ```swift enum IntOrString { case int(Int) case string(String) } extension IntOrString: Decodable { enum CodingKeys: String, CodingKey { case int case string } init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) if container.contains(.int) { self = .int(try container.decode(Int.self, forKey: .int)) } else { self = .string(try container.decode(String.self, forKey: .string)) } } } extension IntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `IntOrString` is a choice element ``` Case 2 refers to decoding implementations of the following form: ```swift init(from decoder: Decoder) throws { self.first = try IntOrString(from: decoder) self.second = try AlternateIntOrString(from: decoder) } ``` Where both `first: IntOrString` and `second: AlternateIntOrString` are choice elements. `AlternateIntOrString` defined as follows: ```swift private enum AlternateIntOrString { case alternateInt(Int) case alternateString(String) } extension AlternateIntOrString: Decodable { enum CodingKeys: String, CodingKey { case alternateInt = "alternate-int" case alternateString = "alternate-string" } init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) if container.contains(.alternateInt) { self = .alternateInt(try container.decode(Int.self, forKey: .alternateInt)) } else { self = .alternateString(try container.decode(String.self, forKey: .alternateString)) } } } extension AlternateIntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `AlternateIntOrString` is a choice element ``` * Add tests * Fix failing tests

## Overview Fixes bug encountered when encoding structs that hold a mixture of choice-element and non-choice-element (or multiple choice-element) properties. ## Example Given a structure that stores both a choice and non-choice property, ```swift private struct IntOrStringAndDouble: Equatable { let intOrString: IntOrString let decimal: Double } ``` the natural encoding approach (now available) is ```swift extension IntOrStringAndDouble: Encodable { enum CodingKeys: String, CodingKey { case decimal } func encode(to encoder: Encoder) { try intOrString.encode(to: encoder) var container = encoder.container(keyedBy: CodingKeys.self) try container.encode(decimal, forKey: .decimal) } } ``` The following `encode` implementation also works: ```swift extension IntOrStringAndDouble: Encodable { enum CodingKeys: String, CodingKey { case decimal } func encode(to encoder: Encoder) { var singleValueContainer = encoder.singleValueContainer() try singleValueContainer.encode(intOrString) var container = encoder.container(keyedBy: CodingKeys.self) try container.encode(decimal, forKey: .decimal) } } ``` `IntOrString` as defined in CoreOffice#119: ```swift enum IntOrString: Equatable { case int(Int) case string(String) } extension IntOrString: Encodable { enum CodingKeys: String, CodingKey { case int case string } func encode(to encoder: Encoder) throws { var container = encoder.container(keyedBy: CodingKeys.self) switch self { case let .int(value): try container.encode(value, forKey: .int) case let .string(value): try container.encode(value, forKey: .string) } } } extension IntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `IntOrString` is a choice element ``` ## Implementation Details In cases where choice and non-choice elements (or multiple choice elements) co-exist in a keyed container, we merge them into a single `XMLKeyedEncodingContainer` (wrapping a `SharedBox<KeyedBox>`). Arrays of choice elements (using `XMLUnkeyedEncodingContainer` under the hood) are encoded the same way as before, as we do not hit the merging cases. For the array case, we still need the `XMLChoiceEncodingContainer` structure. ## Source Compatibility This is an additive change. * Add breaking case * Add choice and keyed merging encode functionality * Refactor * Fix commented code * Fix misnamed file * Fix xcode project * Fix precondition catch * Use switch syntax * Add multiple choice element case * Add explicit types in KeyedBox initialization * Add explicitly empty parameter to KeyedBox initializer * Use more concise type inference * Unify switch syntax * Cut down code duplication * Fix formatting

Mirrors CoreOffice#154 Fixes bugs 1) Decoding multiple choice elements in the same Keyed Container. 2) Decoding choice elements after decoding regular keyed elements in the same container. Case 1 refers to decoding implementations of the form: ```swift init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) otherValue = try container.decode(String.self, forKey: .otherValue) intOrString = try IntOrString(from: decoder) } ``` where `IntOrString` is a choice coding element `IntOrString` defined as follows: ```swift enum IntOrString { case int(Int) case string(String) } extension IntOrString: Decodable { enum CodingKeys: String, CodingKey { case int case string } init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) if container.contains(.int) { self = .int(try container.decode(Int.self, forKey: .int)) } else { self = .string(try container.decode(String.self, forKey: .string)) } } } extension IntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `IntOrString` is a choice element ``` Case 2 refers to decoding implementations of the following form: ```swift init(from decoder: Decoder) throws { self.first = try IntOrString(from: decoder) self.second = try AlternateIntOrString(from: decoder) } ``` Where both `first: IntOrString` and `second: AlternateIntOrString` are choice elements. `AlternateIntOrString` defined as follows: ```swift private enum AlternateIntOrString { case alternateInt(Int) case alternateString(String) } extension AlternateIntOrString: Decodable { enum CodingKeys: String, CodingKey { case alternateInt = "alternate-int" case alternateString = "alternate-string" } init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) if container.contains(.alternateInt) { self = .alternateInt(try container.decode(Int.self, forKey: .alternateInt)) } else { self = .alternateString(try container.decode(String.self, forKey: .alternateString)) } } } extension AlternateIntOrString.CodingKeys: XMLChoiceCodingKey {} // signifies that `AlternateIntOrString` is a choice element ``` * Add tests * Fix failing tests

bwetherfield added 9 commits November 23, 2019 13:24

Add breaking case

869b58e

Add choice and keyed merging encode functionality

afdc196

Refactor

2e2b843

Fix commented code

40f9f42

Fix misnamed file

c335791

Fix xcode project

5d2aeb0

Fix precondition catch

8712a67

Use switch syntax

8ac3944

Add multiple choice element case

fd6ec42

bwetherfield changed the title ~~Mixed choice encoding~~ Mixed choice/non-choice encoding Nov 25, 2019

MaxDesiatov added the bug Something isn't working label Nov 25, 2019

bwetherfield added 4 commits November 25, 2019 16:23

Add explicit types in KeyedBox initialization

63f02cd

Add explicitly empty parameter to KeyedBox initializer

a847719

Use more concise type inference

d27e756

Unify switch syntax

900a530

MaxDesiatov requested changes Nov 26, 2019

View reviewed changes

Cut down code duplication

f5c63ba

houndci-bot reviewed Nov 26, 2019

View reviewed changes

Sources/XMLCoder/Encoder/XMLEncoderImplementation.swift Outdated Show resolved Hide resolved

Sources/XMLCoder/Encoder/XMLEncoderImplementation.swift Outdated Show resolved Hide resolved

Fix formatting

0a9681e

MaxDesiatov approved these changes Nov 27, 2019

View reviewed changes

MaxDesiatov merged commit deb2ab2 into CoreOffice:master Nov 27, 2019

bwetherfield mentioned this pull request Nov 28, 2019

Mixed choice/non-choice decoding #155

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mixed choice/non-choice encoding #154

Mixed choice/non-choice encoding #154

bwetherfield commented Nov 25, 2019

MaxDesiatov Nov 26, 2019

MaxDesiatov Nov 26, 2019

MaxDesiatov Nov 26, 2019

bwetherfield Nov 26, 2019

MaxDesiatov left a comment

Mixed choice/non-choice encoding #154

Mixed choice/non-choice encoding #154

Conversation

bwetherfield commented Nov 25, 2019

Overview

Example

Implementation Details

Source Compatibility

MaxDesiatov Nov 26, 2019

Choose a reason for hiding this comment

MaxDesiatov Nov 26, 2019

Choose a reason for hiding this comment

MaxDesiatov Nov 26, 2019

Choose a reason for hiding this comment

bwetherfield Nov 26, 2019

Choose a reason for hiding this comment

MaxDesiatov left a comment

Choose a reason for hiding this comment