-
Notifications
You must be signed in to change notification settings - Fork 107
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
Dynamic node encoding + new formatters + various fixes #70
Dynamic node encoding + new formatters + various fixes #70
Conversation
Fantastic stuff, thank you very much @JoeMatt! Would it be hard to make this pass CI? Please let me know if you need any help with that. There's also a discussion about enabling XPath queries in #71 , possibly passed in |
@MaxDesiatov Sure no prob, I'll take a look at the CI logs now that the report is generated. I also experimented a bit with using Swift KeyPaths or extensions to CodingKey to make something similar to the XPath solution for nested containers but without the performance issues of XPath like you stated. I'll see what I can come up with since I may need something similar to finish the project I'm using XMLCoder on. |
Interesting to see how |
Codecov Report
@@ Coverage Diff @@
## master #70 +/- ##
==========================================
- Coverage 75.65% 74.42% -1.23%
==========================================
Files 31 37 +6
Lines 1491 1736 +245
==========================================
+ Hits 1128 1292 +164
- Misses 363 444 +81
Continue to review full report at Codecov.
|
…percased Some helpers to deal with converting auto-generated codable String values for instance variables to match some common XML key coding standards to the commonly used Swift camel casing - capitalzied: convert first letter to uppercase - uppercased: All letters uppercased - lowercased: All letters lowercased Support all types that conform to StringProtocol rather than just String
Use a type erased protocl inheritance strategy commonly used to provide default implimentation to avaoid issues with as? checks in generic protocols, while retaining reuse benefits of generic protocols
…y string In the case where a codable provides an empty string for the codable string value for an instance variable an empty bracket was inserted which is invalid XML. ``` let attr = "bar" let value = "FOO" enum CodingKeys : String, CodingKey { case attr case value = "" } ``` Will be useful for unkeyed objects that contain only attributes eg; ```xml <box attr="bar"><>FOO</></box> <!-- Would now correctly become --> <box attr="bar">FOO</box> ```
DynamicNodeEncoding allows easily adding the ability to choose if iVars should be attribute or element encoded by inheriting DynamicNodeEncoding and implimenting a single static function in any Codable class or struct. This is simpler than the current method that requires a global dynamic encoding closure for every XMLEncoder instance. This allows changing the encoding where the data models live, rather than the creator of the XMLEncoder instance needing to have knowledge of all the possible structs and classes that the encoder might encounter at init time.
- refactor element and attribute encoders to closures for easier code reuse - Added type alias for encoding closures for clarity
Had removed them since I was testing intrinsics with attributes. Needed to wrap cateogy value in an element tag again. Also appears the hack for decoding nested arrays is no longer required so removed the complex decoding of Category
Previous version of this test techncially passed on Encdode/Decode comparision sinve the structure values were the same, but the encoding make Book structs id an element, so the strings weren't equal. Modified the simplier single book test to check that the attributes are encoded to XML and match the original string (minus white space formatting)
2a3ae40
to
a110b83
Compare
Just rebased from master and fixed failing tests. This PR branch should be complete as far as my work unless feedback or changes are requested. |
Hey @MaxDesiatov , I found in my testing that I needed the code that did seem redundant as certain tests were failing if I removed the same code as d29ce8f. Maybe this is why you were seeing failing unit tests in #73 |
## Overview This PR fixes #12: decoding of unkeyed single value elements that contain attributes as reported in that issue. ## Example ```xml <?xml version="1.0" encoding="UTF-8"?> <foo id="123">456</foo> ``` ```swift private struct Foo: Codable, DynamicNodeEncoding { let id: String let value: String enum CodingKeys: String, CodingKey { case id case value // case value = "" would also work } static func nodeEncoding(forKey key: CodingKey) -> XMLEncoder.NodeEncoding { switch key { case CodingKeys.id: return .attribute default: return .element } } } ``` Previously this XML example would fail to decode. This PR allows two different methods of decoding discussed in usage. ## Usage This PR will support decoding the example XML in two cases as long as the prerequisite cases are matched. ### Prerequisites 1. No keyed child elements exist 2. Any keyed child nodes are supported Attribute types and are indicated to decode as such ### Supported cases 1. An instance var with the key `value` of any decodable type. 2. An instance var of any key that has a Decoding key of String value "value" or "". The decoder will look for the case where an element was keyed with either "value" or "", but not both, and only one of those values (ie; no other keyed elements). It will automatically find the correct value based on the CodingKey supplied. ## Other considerations The choice to decode as either "value" or "" keys was purely to try to support the inverse to XML version which would only work if an instance var specifically has a `CodingKey` with associated value type `String` that returns an empty string, if PR #70 is commited as-is, which adds XML coding support for unkeyed attributed value elements. The 'value' variant was added as a simpler means to support decoding a nested unkeyed element without having to provide custom CodingKey enum for a struct. Something needed to be provided since Swift doesn't have empty string iVars `let "" : String`, isn't a valid iVar token for example, so `value` was chosen as a logical default. ## Notes This PR is an extension of #70 , though it could be recoded to work off of `master`. The last commit in this PR is the only commit specific to this feature, though #70 provides the inverse solution of creating XML from an attributed value wrapping struct. Coding and decoding unit tests of String and Int values are included.
This adds new `DynamicNodeDecoding` protocol similar to `DynamicNodeEncoding` introduced in #70 * Add DynamicNodeDecoding protocol * Remove NodeDecoding from XMLEncoder * Add NodeDecoding to XMLDecoder * Fix class name in DynamicNodeDecoding.swift * Implement DynamicNodeDecoding with tests * Improve test coverage * Add more example code to README * Fix wording in README * Fix typos, cleanup example code * Cleanup example code in README
Dynamic node encoding
Introduction
The main part of this PR adds a protocol
DynamicNodeEncoding
that attempts to improve the use of custom encoding strategies.This solution is related to #45 , not too dissimilar to @MaxDesiatov RFC in the comments Link. Though this PR is only attempting to resolve the use of node encoding.
This PR includes some commits beyond which are discussed later. Most of those commits could be cherry picked out if rejected though the reasoning for them is hopefully thoroughly explained.
Purpose
The current major shortcoming in XMLEncoder is that the default encoding strategy is to always encode nodes as Elements. In order to override this default, a closure must be passed to each new instance of XMLEncoder, which is only provided the CodingKey path and Encoder instance.
Deep knowledge of all the CodingKey's for each possible encountered struct of class is required in a monolothic switch statement, which makes for difficult refactoring and mostly impossible for dealing with dynamically generated structures or structures with private or otherwise unknown CodingKeys.
This protocol moves the ability for models themselves to determine how they should be translated into XML.
Code
bce416a
Example
Additional changes
Bool encoding
Commit: 05e3c2d
Purpose
Currently bools are only generated from string of exactly
1, 0, true, false
.These are technically the only valid values according to the XML spec, but in Cocoa values of
Yes
andNO
are commonly encountered. The XML spec is trying to dictate type inference, but since Swift is statically typed, and we're only matching to a known key type, it's of my opinion that it's within reason to allow a programmer to convert non-conforming Boolean like String values to Swift Bool's without requiring custom encode functions.This commit allows for:
yes, no, y, 'n
Additional default key encodings
5dbe1d7
Purpose
Default convenience of common XML key casings in addition to the current snake case key conversion.
This commit also changes the extensions on
String
toStringProtocol
to allow for more flexibility on code reuse.SharedBoxProtocol: Generalize for any Box inheritance
9ae94dc
Purpose
SharedBox
has a generic typeUnboxed:Boxed
but the default implementation offunc unbox()
returns the concrete protocol type Box, instead of the generic inherited type ofUnboxed
. This requires some additional type checking in other parts of the code.It additionally adds a type removed protocol with a default implementation of
func unbox()
that retains the current behavior of returning the non-generic protocol, to avoidif let
s that don't work with protocols with associated types, but where simply conforming toBox
protocol is all that is required.This overall cleans up some the code and removes some fail() calls and type checking in a later commit.
Empty string key value encoding fix
551d484
Purpose
In the case where a codable provides an empty string for the codable string value for an instance variable an empty bracket was inserted which is invalid XML.
Will be useful for unkeyed objects that contain only attributes eg;
This is related to #12 , this is the encoding case. I'm still working on another PR for the decoding of un-keyed intrinsics with attributes
Additional considerations
In the event that more than 1 element exists, this will still produce what is most likely invalid XML. It's possible that this solution would need to be extended to throw if any other elements exist in past or future element encodings.
XMLEncoder: Add both option to value encoding, refactor encoder
bd24a08
Purpose
Similar to the case of #45, where an attribute and element have the same key value, this allows encoding the same value in both places.
This is an kind of an odd case, but in real world use of XMLCoder, I've run into a SOAP specification where XML elements in the payload have the same value in both an attribute and element position. Adding a both optional allowed using the DynamicNodeEncoding protocols still instead of requiring a custom encoding function for the model with duplicated attribute/element.
It refactors the complex switch statement slightly as well to use local closures for better readability and reusing the same code for the both case. It also adds some type aliases on the closures used in attribute encoding to make editing and reading a little easier.
Fix warnings
5eedcea
Purpose
Swift lint was spitting out a lot of warnings presently.
Resolved:
This allows for 1, the Swift compiler to auto-generate the default initializer (all custom init's and conformance to protocols that auto-generate inits such as Codable need to be in extensions in order for the Swift compiler to auto-generate the default initializers. That default initializer can then be called in additional initializers in extensions which helps reduce complexity in refactoring, adding, removing instance variables to those structs.
Unresolved:
XMLCoderElement.flatten()
still warns about complexity, but it was slightly improved by converting a mutable array builder to a immutable reduce() function that's more Swifty. It should be a little easier now to refactor the reduce closure to something less complex.