Skip to content
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

XmlEncoder: ordering of elements #17

Closed
Ma-He opened this issue Dec 15, 2018 · 11 comments
Closed

XmlEncoder: ordering of elements #17

Ma-He opened this issue Dec 15, 2018 · 11 comments
Assignees

Comments

@Ma-He
Copy link

Ma-He commented Dec 15, 2018

Hi,
is there any possibility to set the order of attributes in an element while encoding a model-instance?
I'm trying to build a GPX reader/writer with this little awesome framework and have noticed that the order of my attributes in an element varies from encode to encode. Am I missing something?

Thanks in advance

@MaxDesiatov
Copy link
Collaborator

Hi @Ma-He, thank you reporting this issue. There's a .sortedKeys option that can be set for XMLEncoder.outputFormatting property.

We plan to publish auto-generated docs for XMLCoder soon, which would make this option more visible to users.

Also, if you're using this option, you might need to switch to master branch as it was recently reported that this option was broken. This was fixed in #11 and #14 and merged to master.

@regexident
Copy link
Contributor

Sorting is not was OP is looking for. Ordering is. That is <foo>…</foo> has to appear before <bar>…</bar>, if .foo was encoded before .bar.

This is not supported currently as XMLCoder uses unordered NSDictionary in its internal keyed encoding container.

@MaxDesiatov
Copy link
Collaborator

Thanks @regexident, I understand now. Still, my understanding was that the main concern is about determinism. I hope that .sortedKeys is able to solve that. I guess we have to transition to a tuple array for internal storage to maintain order as well.

@regexident
Copy link
Contributor

regexident commented Dec 15, 2018

I'm fairly certain as writing a Codable-powered GPX implementation was my reason for dabbling with XMLCoder (or rather it's predecessor XMLParser) as well.

GPX (like many other XML schemas) expects certain keys to have a specific order:

Search for <!-- elements must appear in this order --> in the GPX 1.1 Schema.

This requirement of specific order also applies to elements, not just attributes.

@Ma-He
Copy link
Author

Ma-He commented Dec 16, 2018

Hey guys,

thanks for your quick responses. As @regexident said, ordering it is what I`m looking for :-)

MaxDesiatov pushed a commit that referenced this issue Dec 20, 2018
)

This PR removes any remaining use of `NS…Array`/`NS…Dictionary`/`NSNumber`/`NSDecimalNumber`/`NSNull`/… (as storage) from the code-base.

It introduces an internal type …

```swift
internal enum Box {
    case null(NullBox)
    case bool(BoolBox)
    case decimal(DecimalBox)
    case signedInteger(SignedIntegerBox)
    case unsignedInteger(UnsignedIntegerBox)
    case floatingPoint(FloatingPointBox)
    case string(StringBox)
    case array(ArrayBox)
    case dictionary(DictionaryBox)
}
```

… as well as accompanying variant box types, replacing use of `Any`/`NS…`.

👷🏻‍♀️It improves type-safety by reducing use of `Any` (from 60 down to 19 occurrences) as well as `NSObject` (from 37 down to 1 occurrence).

🏗It further more levels the field for improvements/additions such as [support for order-preserving elements & attributes](#17).

💡Thanks to encapsulation we can now safely change the inner logic of `DictionaryBox` to retain insertion order.

**Edit**:

We ended up replacing aforementioned `enum Box` with a protocol:

```swift
protocol Box {
    var isNull: Bool { get }
    var isFragment: Bool { get }
    
    func xmlString() -> String?
}
```

* Added basic box types
* Migrated decoding logic to proper boxes
* Migrated encoding logic to proper boxes
* Moved auxiliary types into their own dedicated files and group
* Replaced use of `.description` with explicit dedicated `.xmlString`
* Made box types conform to `Equatable`
* Added basic unit tests for box types
* Removed manual `Equatable` conformance, changed fragment boxes to structs
* Removed redundant use of `internal` on internal types
* Simplified logic of `func createElement` instances
* Removed `enum Box` in favor of `protocol Box`
* Fixed error description for `.typeMismatch`
* Shortened type names of `FloatingPointBox`, `SignedIntegerBox` and `UnsignedIntegerBox`
* Moved unboxing logic into dedicated Box initializers
* Removed last remaining use of explicit `internal` related to this PR
* Changed `Unboxed` types of `IntBox` & `UIntBox` to explicit 64bit types
* Made `.xmlString` conform to W3.org’s XML-spec
* Renamed `.init(string:…)` to `.init(xmlString:…)`
* Implemented conformity to encoding strategies
* Simplified box unit tests
* Added minimalist decoding unit tests for individual box types
* Fixed bug related to `Float` encoding and simplified internal encoding/decoding locig
@regexident
Copy link
Contributor

regexident commented Dec 22, 2018

Nitpick: It's not the order of attributes that's unsupported, it's the order of elements.

The XML specification explicitly defines the order of attributes to be of no importance:

The Name in the start- and end-tags gives the element's type. [Definition: The Name-AttValue pairs are referred to as the attribute specifications of the element], [Definition: with the Name in each pair referred to as the attribute name ] and [Definition: the content of the AttValue (the text between the ' or " delimiters) as the attribute value.] Note that the order of attribute specifications in a start-tag or empty-element tag is not significant. (emphasis mine) Source

As such NSXMLParser (which XMLCoder is based on) doesn't even provide a way to retrieve an element's attributes in the order of appearance.

@MaxDesiatov
Copy link
Collaborator

MaxDesiatov commented Dec 23, 2018

TLDR: We're slowly working towards ordered child elements, but not quite sure if ordering of attribues will be possible in the current implementation.

@Ma-He could you please clarify if you're looking for ordering of XML attributes or child elements?

@MaxDesiatov
Copy link
Collaborator

If you're specifically interested in supporting GPX, looks like it only requires ordered elements, not attributes.

@Ma-He
Copy link
Author

Ma-He commented Jan 4, 2019

@MaxDesiatov you`re right, just the elements

MaxDesiatov pushed a commit that referenced this issue Jan 13, 2019
Unifies the representation of `T` where `T: Box` and makes it explicit where shared reference semantics is expected.

With this code performance is consistently 5-30% worse, could this be caused by copies of value types? Can this be improved when Swift 5.0 is released or whatever the version is that allows us to improve performance of value types? After reviewing related issues we'd prefer XMLCoder to be slightly slower, but with #12, #17 and #25 fixed than the opposite 😞 

* Added `SharedBox` type for on-demand reference semantics
* Remove `print` that kills performance tests
* Handle all cases with SharedBox containers
* Add RJITest back to the project file, fix the test
* Remove unused internal initializer
@CineDev
Copy link

CineDev commented Feb 12, 2019

I have to say you doing great job!
But current iteration of XMLEncoder is basically broken for use cases where the ordering is important. I tried to use it in my simple text editor. I'm storing each paragraph individually in my model. But from save to save my model xml representation gets shuffled, so the text information in the saved document become completely useless.
I hope, the ordering is on the way.

@MaxDesiatov MaxDesiatov changed the title XmlEncoder: ordering of attributes in an element XmlEncoder: ordering of elements Feb 27, 2019
@MaxDesiatov MaxDesiatov self-assigned this Feb 27, 2019
MaxDesiatov added a commit that referenced this issue Mar 1, 2019
Resolves #17 

* First attempt at ordered encoding
* Fix line length linter warning
* Rename DynamicNodeEncoding function, fix order
@MaxDesiatov
Copy link
Collaborator

MaxDesiatov commented Mar 1, 2019

@Ma-He @CineDev this is now fixed in master, no source-breaking API changes, should work out of the box. .sortedKeys now only has effect on attributes, not attributes and elements like before.

bwetherfield added a commit to bwetherfield/XMLCoder that referenced this issue Jul 17, 2019
bwetherfield added a commit to bwetherfield/XMLCoder that referenced this issue Jul 26, 2019
bwetherfield pushed a commit to bwetherfield/XMLCoder that referenced this issue Jul 26, 2019
Add benchmark baselines

Pull in tests (CoreOffice#11)

Add Decoding support for choice elements (CoreOffice#15)

Fix indentation (CoreOffice#16)

Replace usage of XCTUnrwap (CoreOffice#19)

Add nested choice tests (CoreOffice#18)

Add falsely passing double array roundtrip test (CoreOffice#17)
bwetherfield pushed a commit to bwetherfield/XMLCoder that referenced this issue Jul 27, 2019
Add benchmark baselines

Pull in tests (CoreOffice#11)

Add Decoding support for choice elements (CoreOffice#15)

Fix indentation (CoreOffice#16)

Replace usage of XCTUnrwap (CoreOffice#19)

Add nested choice tests (CoreOffice#18)

Add falsely passing double array roundtrip test (CoreOffice#17)
bwetherfield added a commit to bwetherfield/XMLCoder that referenced this issue Jul 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants