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

Can’t reach an XML value #12

Closed
qmoya opened this issue Dec 2, 2018 · 8 comments · Fixed by #73
Closed

Can’t reach an XML value #12

qmoya opened this issue Dec 2, 2018 · 8 comments · Fixed by #73
Assignees

Comments

@qmoya
Copy link
Contributor

qmoya commented Dec 2, 2018

Hi there! I can’t find the right data structure to decode the following XML string:

<something name="Some name">Some value</something>

I ran out of ideas. May anyone give me a hint?

Thanks!

@MaxDesiatov MaxDesiatov assigned MaxDesiatov and unassigned hodovani Jan 11, 2019
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
JoeMatt added a commit to salido/XMLCoder that referenced this issue Feb 6, 2019
Add intrinsic encoding decoding with attributes support
JoeMatt added a commit to salido/XMLCoder that referenced this issue Feb 6, 2019
Add intrinsic encoding decoding with attributes support
JoeMatt added a commit to salido/XMLCoder that referenced this issue Feb 6, 2019
Add intrinsic encoding decoding with attributes support
JoeMatt added a commit to salido/XMLCoder that referenced this issue Feb 19, 2019
Add intrinsic encoding decoding with attributes support
JoeMatt added a commit to salido/XMLCoder that referenced this issue Feb 21, 2019
JoeMatt added a commit to salido/XMLCoder that referenced this issue Feb 22, 2019
MaxDesiatov pushed a commit that referenced this issue Feb 22, 2019
## 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.
@MaxDesiatov
Copy link
Collaborator

This is now supported in master with #73 merged. As usual please feel free to reopen this issue if you think there's anything missing.

@thecb4
Copy link

thecb4 commented Feb 22, 2019

Trying to use master with the following and still getting an error

<preview_image_time format="24/999 1000/nonDrop">00:00:17:01</preview_image_time>


struct PreviewImageTime: Codable, DynamicNodeEncoding {
  var format: String
  var value: String

  enum CodingKeys: String, CodingKey {
    case format
    case value = ""
  }

  static func nodeEncoding(forKey key: CodingKey) -> XMLEncoder.NodeEncoding {
    switch key {
      case CodingKeys.format:
        return .attribute
      default:
        return .element
    }
  }
}

could not parse
typeMismatch(Swift.Dictionary<Swift.String, Any>, Swift.DecodingError.Context(codingPath: [CodingKeys(stringValue: "software", intValue: nil), CodingKeys(stringValue: "software_metadata", intValue: nil), CodingKeys(stringValue: "versions", intValue: nil), CodingKeys(stringValue: "version", intValue: nil), XMLKey(stringValue: "0", intValue: 0), CodingKeys(stringValue: "locales", intValue: nil), CodingKeys(stringValue: "locale", intValue: nil), XMLKey(stringValue: "0", intValue: 0), CodingKeys(stringValue: "app_previews", intValue: nil), CodingKeys(stringValue: "app_preview", intValue: nil), XMLKey(stringValue: "0", intValue: 0), CodingKeys(stringValue: "preview_image_time", intValue: nil)], debugDescription: "Expected to decode Dictionary<String, Any> but found SharedBox instead.", underlyingError: nil))

@JoeMatt
Copy link
Contributor

JoeMatt commented Feb 22, 2019

@thecb4 What happens if you change your string values to something without colons or backslashes.

I think I ran into something similar with UUID strings that had colons. I believe the decoder is attempting to decode a dictionary before a string, based on the ability to convert "k : v " strings to dictionaries.

If that's a case, a workaround may be to provide your own decoder that specifies the type to try.

I would just try a simple test case first with simple strings to see if that is indeed the issue. I have some example code that might help if that is it.

@thecb4
Copy link

thecb4 commented Feb 23, 2019

Now I understand the error message! Perfect.

@MaxDesiatov
Copy link
Collaborator

hm, I don't think the attempt to decode that string as a dictionary is something anyone expects to be a default behavior. @thecb4 I hope you don't mind if I add that XML snippet to XMLCoder test suite

@thecb4
Copy link

thecb4 commented Feb 23, 2019

Thank you. I have been trying to figure out the appropriate work around. I tried a custom init and was still getting the same error.

struct PreviewImageTime: Codable {

  var format: String
  var value: String

  public init(from decoder: Decoder) throws {

      var values = try decoder.container(keyedBy: CodingKeys.self)

      var unkeyedValues = try values.nestedUnkeyedContainer(forKey: .value) as UnkeyedDecodingContainer

      format = try values.decode(String.self, forKey: .format)
      value = try unkeyedValues.decode(String.self, forKey: .value)

  }

  enum CodingKeys: String, CodingKey {
    case format
    case value = ""
  }

  static func nodeEncoding(forKey key: CodingKey) -> XMLEncoder.NodeEncoding {
    print("node encoding")
    switch key {
      case CodingKeys.format:
        return .attribute
      default:
        return .element
    }
  }
}

@thecb4
Copy link

thecb4 commented Feb 23, 2019

FYI - When I create a standalone XML element, it works. When it's embedded in another element. it fails.

// This passes
  let previewTime = 
  """
  <?xml version="1.0" encoding="UTF-8"?>
  <preview_image_time format="24/999 1000/nonDrop">00:00:17:01</preview_image_time>
  """.data(using: .utf8)!


  let decoder = XMLDecoder()
  decoder.errorContextLength = 10

  let metadata1 = try decoder.decode(PreviewImageTime.self, from: previewTime)
  print(metadata1)

// this does not pass
  let preview = 
  """
  <?xml version="1.0" encoding="UTF-8"?>
  <app_preview display_target="iOS-6.5-in" position="1">
    <preview_image_time format="24/999 1000/nonDrop">00:00:17:01</preview_image_time>
  </app_preview>
  """.data(using: .utf8)!

  let metadata2 = try decoder.decode(AppPreview.self, from: preview)
  print(metadata2)

structs

struct AppPreview: Codable {
  var displayTarget: String
  var position: Int
  var previewImageTime: PreviewImageTime
  // var dataFile: DataFile

  enum CodingKeys: String, CodingKey {
    case displayTarget = "display_target"
    case position
    case previewImageTime = "preview_image_time"
    // case dataFile = "data_file"
  }
}

struct PreviewImageTime: Codable, DynamicNodeEncoding {

  var format: String
  var value: String

  enum CodingKeys: String, CodingKey {
    case format
    case value
  }

  static func nodeEncoding(forKey key: CodingKey) -> XMLEncoder.NodeEncoding {
    print("node encoding")
    switch key {
      case CodingKeys.format:
        return .attribute
      default:
        return .element
    }
  }
}

@MaxDesiatov
Copy link
Collaborator

Thank you @thecb4, that's very helpful! Clearly looks like a bug to me, looking into it right now.

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

Successfully merging a pull request may close this issue.

5 participants