-
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
Attributed Intrinsic (value coding key) #73
Attributed Intrinsic (value coding key) #73
Conversation
…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)
Add intrinsic encoding decoding with attributes support
94b841f
to
559f35a
Compare
It's incredibly difficult to find the failing error i the Travis logs. I'd suggest turning off the verbosity, and if linting is going to be a failure it might be faster to incorporate it as a pre-commit hook or use Danger or something that's faster than a full travis build for non-compilation related failures. |
@MaxDesiatov Interesting, I don't think they were failing on my branch. I'll try to take a look this week. I'm moving this code into my own SOAP Swift Coding library that depends on XMLCoder so I'll have to fix any issues that arise. I'll push up fixes as I find them. |
@JoeMatt while I've tried to remove that |
Any news here? I'm definitely interesting in using this to support AppStore Connect uploading. |
A few days I’ll be back on this. Need to fix something in another dependency of my app first. |
Hello, any updates on this? |
I'm testing / fixing a case where the attributes are enums with associated values that can be reduced to a simple box. Also, it's ready when it's ready, no need to keep posting for updates. Subscribe to the issue to get notified on updates. |
Thanks @JoeMatt, no worries! I was thinking if I should pick up the branch myself, I wonder if any of the commenters were interested in contributing or planning their own work around this given that it's a pressing issue. Please let me know if there is any way I can help. |
hm, most of the linter warnings reported by @houndci-bot are not valid, it probably didn't pick up the actual config. Sorry about that, it was setup just recently. I'll resolve irrelevant warnings |
also, merged |
Codecov Report
@@ Coverage Diff @@
## master #73 +/- ##
==========================================
+ Coverage 74.4% 74.65% +0.24%
==========================================
Files 37 37
Lines 1735 1752 +17
==========================================
+ Hits 1291 1308 +17
Misses 444 444
Continue to review full report at Codecov.
|
@MaxDesiatov I have one more thing I'm working on. Doesn't need to be in this PR. example: <number type="unit">A201</number> public enum PostingType : Equatable, Codable {
private enum CodingKeys: String, CodingKey {
case unit
case resno
}
public init(from decoder: Decoder) throws {
let values = try decoder.container(keyedBy: CodingKeys.self)
if let unit = try values.decodeIfPresent(String.self, forKey: .unit) {
self = .unit(unit)
return
} else {
let resno = try values.decode(String.self, forKey: .resno)
self = .resno(resno)
return
}
}
public func encode(to encoder: Encoder) throws {
var container = encoder.container(keyedBy: CodingKeys.self)
switch self {
case .unit(let unit):
try container.encode(unit, forKey: .unit)
case .resno(let resno):
try container.encode(resno, forKey: .resno)
}
}
case unit(String)
case resno(String)
}
public struct PostingNumber : VendorDataProtocol, DynamicNodeEncoding {
public let type : PostingType
public init(type: PostingType) {
self.type = type
}
enum CodingKeys : String, CodingKey {
case type
case typeValue = ""
}
public static func nodeEncoding(forKey key: CodingKey) -> XMLEncoder.NodeEncoding {
switch key {
case PostingNumber.CodingKeys.type: return .attribute
default: return .element
}
}
public init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)
let type = try container.decodeIfPresent(PostingType.self, forKey: .type)
let typeArray = try container.decodeIfPresent([PostingType].self, forKey: .type) // Deal with the double value of type as attribute and element
if let type = type {
self.type = type
} else if let typeArray = typeArray, let first = typeArray.first {
self.type = first
} else {
throw DecodingError.keyNotFound(CodingKeys.type, DecodingError.Context.init(codingPath: decoder.codingPath, debugDescription: "Missing type"))
}
}
public func encode(to encoder: Encoder) throws {
var container = encoder.container(keyedBy: CodingKeys.self)
switch type {
case .unit(let unit):
try container.encode("unit", forKey: .type)
try container.encode(unit, forKey: .typeValue)
case .resno(let resno):
try container.encode("resno", forKey: .type)
try container.encode(resno, forKey: .typeValue)
}
}
} |
looks like all checks pass, thank you very much @JoeMatt! I'll review this in the next few days |
} | ||
} | ||
|
||
// TODO: Fix decoding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo Violation: TODOs should be resolved (Fix decoding). (todo)
I pushed one more commit which is the codable test for associated type unkeyed intrinsics. The decoding I couldn't get to work. Requires more knowledge of the decoding code. I tried adding deifference decoders with code like func unbox<T: Decodable & RawRepresentable>(_ box: Box) throws -> T where T.RawValue: Decodable { ...
T.init(rawValue:
} No dice, and that only covers enums that are RawRepresentable. Associated value types I have no idea how to do or if it's even possible in Swift 4.2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again, LGTM in general! We try to avoid print
calls in tests as those slow down test execution and you can attach printing breakpoints during debugging anyway. I also think we don't need to catch errors in tests explicitly as those are rethrown and reported automatically by XCTest. I've added those changes myself to quickly merge the PR as I see a few people would find it handy merged sooner.
Looking forward to the DynamicNodeDecoding
PR if there's anything like that in the works.
Cool, thanks a lot @MaxDesiatov . Sorry for leaving in those prints. I have a habit of putting them everywhere in my unit tests but in my local setup, they get shuffled to a console logging app. XMLCoder is really coming along! If I see anything else in my code that could be of use I'll definitely make more PRs. |
## Overview Restricts the special case usage of `"value"` OR `""` as a coding key, as implemented in #73 to just `""`. `"value"` resumes functioning as a normal coding key. ## Example As noted in #145, `"value"` is starting to collide awkwardly in one or two cases where it may be _implicit_ per the special case usage ```swift <value-containing>I am implicitly labeled</value-containing> ``` or _explicit_, as in ```swift <value-containing> <value>I am explicitly labeled</value> </value-containing> ``` With the change proposed, the latter example will be unambiguously captured by ```swift struct ValueContaining: Codable { let value: String } ``` while the former is equivalent to ```swift struct ValueContaining: Codable { let value: String enum CodingKeys: String, CodingKey { case value = "" } } ``` ## Source Compatability This is a **breaking change** for downstream users of the special `"value"` coding key. All such uses should be replaced by `case value = ""` subject to this PR's inclusion.
## Overview Restricts the special case usage of `"value"` OR `""` as a coding key, as implemented in CoreOffice#73 to just `""`. `"value"` resumes functioning as a normal coding key. ## Example As noted in CoreOffice#145, `"value"` is starting to collide awkwardly in one or two cases where it may be _implicit_ per the special case usage ```swift <value-containing>I am implicitly labeled</value-containing> ``` or _explicit_, as in ```swift <value-containing> <value>I am explicitly labeled</value> </value-containing> ``` With the change proposed, the latter example will be unambiguously captured by ```swift struct ValueContaining: Codable { let value: String } ``` while the former is equivalent to ```swift struct ValueContaining: Codable { let value: String enum CodingKeys: String, CodingKey { case value = "" } } ``` ## Source Compatability This is a **breaking change** for downstream users of the special `"value"` coding key. All such uses should be replaced by `case value = ""` subject to this PR's inclusion.
Overview
This PR fixes #12: decoding of unkeyed single value elements that contain attributes as reported in that issue.
Example
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
Supported cases
value
of any decodable type.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 typeString
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, sovalue
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.