XMLCoder icon indicating copy to clipboard operation
XMLCoder copied to clipboard

Extra level of indirection?

Open comiclandapp opened this issue 5 years ago • 13 comments

Moving away from NSXMLParser (Objective-c) in my app to Decodable makes this library very handy. Thank you for forking and maintaining it.

I have a Goodreads XML response which I'm decoding with XMLCoder. That works great.

goodreads.xml.txt Goodreads.swift.txt GoodreadsTests.swift.txt

However, there is a level of indirection after following the books example which is bothering me a bit.

In the Book class I need to add this:

var authors: AuthorList

instead of this:

var authors: [Author]

where AuthorList is defined as:

struct AuthorList: Decodable {
  var authors: [Author]
  enum CodingKeys: String, CodingKey {
    case authors = "author"
  }
}

If I don't do that, I get a parsing error:

keyNotFound(CodingKeys(stringValue: "id", intValue: nil), Swift.DecodingError.Context(codingPath: [CodingKeys(stringValue: "book", intValue: nil), CodingKeys(stringValue: "authors", intValue: nil), _XMLKey(stringValue: "Index 0", intValue: 0)], debugDescription: "No value associated with key CodingKeys(stringValue: \"id\", intValue: nil) (\"id\").", underlyingError: nil))
/Users/tony/Downloads/XMLCoder-master/Tests/XMLCoderTests/GoodreadsTests.swift:24: error: -[XMLCoderTests.GoodreadsTests testGoodreadsBundle] : XCTAssertNotNil failed - error retrieving book

When using var authors: [Author] I added a case to codingKeys in Book:

case authors = "author"

to no avail.

Of course the hack forces me to do this in the test case:

let authors = book?.authors.authors.map { $0 }

instead of this:

let authors = book?.authors.map { $0 }

Any hints to remove this extra level of indirection would be very much appreciated.

Thanks a lot.

comiclandapp avatar Nov 25 '18 16:11 comiclandapp

I have the same issue with the following xml snippet:

<players>
    <player>
      <capnumber>1</capnumber>
      <firstname>a</firstname>
      <lastname>ASDF</lastname>
      <birthday />
      <license>123</license>
    </player>
  </players>

galli-leo avatar Dec 08 '18 19:12 galli-leo

In a PR I just submitted #70 , commit bce416aa2d295f2f5e0d7ac53cc1aa11d0675162 has an example of a custom encoder function that deals with an array of shared keys.

I simplified it a bit using an extension on KeyedDecodingContainer (required to get the CodingKey type of the container)

To use @galli-leo 's example it would look something like this,

extension KeyedDecodingContainer {
    func decodeArray<T: Decodable>(of type: T.Type, forKey key: KeyedDecodingContainer<K>.Key) throws -> [T] {
        var nested = try nestedUnkeyedContainer(forKey: key)

        var decoded = [T]()

        while !nested.isAtEnd {
            do {
                if let another = try nested.decodeIfPresent(T.self) {
                    decoded.append(another)
                }
            } catch DecodingError.valueNotFound {
                continue
            } catch {
                throw error
            }
        }

        return decoded
    }
}

struct Team: Codable {
    let name: String
    let players: [Player]

    private enum CodingKeys: String, CodingKey {
        case name
        case players = "player"
    }
}

extension Team {
    init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        name = try container.decode(String.self, forKey: .name)
        players = try container.decodeArray(of: Player.self, forKey: .players)
    }
}

struct Player: Codable {
    let capnumber: Int
    let firstname: String
    let lastname: String
    let birthdate: Date?
    let license: String
}

edit:

I'm going to leave this for reference since I already coded and tested this, but it appears in the latest head of master this is no longer required. Decoding of arrays with the same key seems to work automatically now.

This should be marked closed?

JoeMatt avatar Jan 31 '19 09:01 JoeMatt

Keeping this open until we have the XPath idea floating around clarified and possibly implemented. When (if) we do have it ready, that would resolve this specific issue in a much easier way if I understand correctly.

MaxDesiatov avatar Feb 24 '19 19:02 MaxDesiatov

Hi @JoeMatt. Can you explain a bit more about what you commented:

Decoding of arrays with the same key seems to work automatically now.

I have the following XML:

<trips>
  <trip id="1" seq="1">
    ...
  </trip>
  <trip id="1" seq="1">
    ...
  </trip>
</trips>

I found myself doing the following to decode it:

struct Response: Codable {
    let trips: Trip.Wrapper<[Trip]>
}

Where the Trip declaration is as follows:

struct Trip: Codable {
    let id: Int
    let seq: Int
    ...
}

extension Trip {
    struct Wrapper<T>: Codable where T: Codable {
        let trip: T
    }
}

Then, if I have to get the id of the first trip I have to do:

response.trips.trip.first?.id

Is there any other better way to decode this XML?

What would be ideal is:

response.trips.first?.id

Thanks!

acecilia avatar Jul 04 '19 14:07 acecilia

I would have assumed this would work no?

typealias Trips = Array<Trip>
struct Response: Codable {
    let trips: Trips
}

JoeMatt avatar Jul 08 '19 16:07 JoeMatt

@JoeMatt I just tested it in version 0.7. No, it does not work. I do not think the XMLCoder can infer the trip tag name from the trips CodingKey

acecilia avatar Jul 08 '19 16:07 acecilia

Try this,

typealias Trips = Array<Trip>
struct Response: Codable {
    let trips: Trips

     private enum CodingKeys: String, CodingKey {
         case trips = "trip"
     }
}

extension Team {
    init(from decoder: Decoder) throws {
       /// Single value decoder
    }
}

I can't remember the code for a single value decoder off the top of my head, but that might work. I think that's what I was writing in my first example (I barely remember writing it at all), but this line stands out, var nested = try nestedUnkeyedContainer(forKey: key)

JoeMatt avatar Jul 08 '19 16:07 JoeMatt

I would also recommend using different data models structs for decoding and your business logic models. In my experience trying to keep them the same results in writing complex extensions to decodable 😆 And to what ends? response.trips.trip.first?.id might be annoying to type, but you only need it once in a factory transformer to you business logic structs.

You might be able to use some trickery with subscript or @dynamicCallable to skip through that single istance var as well.

JoeMatt avatar Jul 08 '19 16:07 JoeMatt

@JoeMatt Yes, that is what I am doing: this is a model in my data layer. Due to your comment, I was just wondering if there was a simple way of removing that extra level that I missed. But seems that the only way is to manually implement Codable, which as you say is a bunch of boilerplate code.

acecilia avatar Jul 08 '19 16:07 acecilia

For having it as a reference for the future, this is how I end up partially solving this issue in a way that is reusable:

import Foundation

/// A struct used to decode an array of data,
/// where the decoding key is the singular value of the previous key in the coding path
struct NestedArray<T>: Codable where T: Codable {
    private struct CodingKeys: CodingKey {
        var stringValue: String
        var intValue: Int?

        init(stringValue: String) {
            self.stringValue = stringValue
        }

        init?(intValue: Int) {
            self.intValue = intValue
            stringValue = "\(intValue)"
        }

        init(_ codingPath: [CodingKey]) throws {
            guard let previousKey = codingPath.last?.stringValue else {
                throw NestedArrayCodingError.previousKeyNotFound(codingPath)
            }

            let keyValue: String
            if previousKey.hasSuffix("ies") {
                keyValue = previousKey.dropLast(3).appending("y")
            } else if previousKey.hasSuffix("s") {
                keyValue = String(previousKey.dropLast(1))
            } else {
                throw NestedArrayCodingError.couldNotGenerateSingularKeyFromPlural(previousKey, codingPath)
            }
            self = CodingKeys(stringValue: keyValue)
        }
    }

    var value: [T]

    init(from decoder: Decoder) throws {
        let codingKey = try CodingKeys(decoder.codingPath)
        let container = try decoder.container(keyedBy: CodingKeys.self)
        value = try container.decode([T].self, forKey: codingKey)
    }

    func encode(to encoder: Encoder) throws {
        let codingKey = try CodingKeys(encoder.codingPath)
        var container = encoder.container(keyedBy: CodingKeys.self)
        try container.encode(value, forKey: codingKey)
    }
}

private enum NestedArrayCodingError: Error, CustomDebugStringConvertible {
    case previousKeyNotFound(_ codingPath: [CodingKey])
    case couldNotGenerateSingularKeyFromPlural(_ previousKey: String, _ codingPath: [CodingKey])

    var debugDescription: String {
        switch self {
        case let .previousKeyNotFound(codingPath):
            return """
            There is no previous key to use as reference for decoding
            CodingPath: \(codingPath)
            """

        case let .couldNotGenerateSingularKeyFromPlural(previousKey, codingPath):
            return """
            The previous key could not be converted to singular
            PreviousKey: \(previousKey)
            CodingPath: \(codingPath)
            """
        }
    }
}

With that you can do:

struct Response: Codable {
    let trips: NestedArray<Trip>
}

And obtain the first value of the trips array as:

response.trips.value.first?.id

acecilia avatar Jul 08 '19 21:07 acecilia

This is a fantastic idea @acecilia. This probably could look even better with property wrappers and key path dynamic members in Swift 5.1 that allow addressing wrapped properties directly 👍

MaxDesiatov avatar Jul 09 '19 12:07 MaxDesiatov

Following up on your Swift 5.1 comment -- this works great for me using dynamic member lookup, however I wasn't able to generify it yet. I also tried this only with decoding, I'd guess there's more work to get it to encode correctly:

@dynamicMemberLookup
struct Container: Decodable {
    private var itemList: ItemList

    enum CodingKeys: String, CodingKey {
        case itemList = "items"
    }

    subscript<T>(dynamicMember keyPath: KeyPath<ItemList, T>) -> T {
        return itemList[keyPath: keyPath]
    }
}

struct ItemList: Decodable {
    let items: [Item]

    enum CodingKeys: String, CodingKey {
        case items = "item"
    }
}

That could decode

<container>
  <items>
    <item>...</item>
    <item>...</item>
  </items>
</container>

And still allows me to nicely access the list through just container.items.

xfxian avatar May 20 '21 21:05 xfxian

I found a short solution that works in our app by reading this thread. So in case it's useful for someone here it is:

@propertyWrapper
public struct NestedArray<Value: Decodable>: Decodable {

    public var wrappedValue: [Value] = []

    public init(from decoder: Decoder) throws {
        var container = try decoder.unkeyedContainer()

        guard let count = container.count else { return }

        for _ in 0..<count {
            let value = try container.decode(Value.self)
            wrappedValue.append(value)
        }
    }

    public init(wrappedValue: [Value]) {
        self.wrappedValue = wrappedValue
    }
}

It can be used in the following structs to decode the following XML.

struct Session: Decodable {
    let key: String
    @NestedArray var files: [File]
}

struct File: Decodable {
    let name: String
    let link: String
}
<?xml version="1.0" encoding="UTF-8"?>
   <session>
     <key>1234</key>
     <files>

       <file>
         <name>File1</name>
         <link>Link1</link>
       </file>

       <file>
         <name>File2</name>
         <link>Link2</link>
       </file>

     </files>
   </session>

ABridoux avatar Nov 23 '21 16:11 ABridoux