wvteijlingen / Spine

A Swift library for working with JSON:API APIs. It supports mapping to custom model classes, fetching, advanced querying, linking and persisting.
MIT License
266 stars 109 forks source link

Added support for extracting attributes from a key path #89

Closed invliD closed 8 years ago

invliD commented 8 years ago

Imagine a JSON-API resource as follows:

{
  "data": {
    "type": "videos",
    "id": "123",
    "attributes": {
      "title": "Example Video",
      "stream": {
        "hd-url": "…",
        "sd-url": "…",
        "hls-url": "…",
      }
    }
  }
}

It is currently impossible to directly retrieve the attributes from the nested object stream. With this PR, the serialized name of an Attribute can be specified in the "key path" syntax, in this case an Attribute definition could look like this: "stream_hd_url": Attribute().serializeAs("stream.hls-url").

This implementation only handles deserialization, but I could add serialization as well if necessary.

wvteijlingen commented 8 years ago

Thanks for the PR! I'm not that keen on merging this right now actually. You can already deserialise nested objects by using a custom transformer.

invliD commented 8 years ago

@wvteijlingen: I assume by "transformer" you mean a ValueFormatter? Can you give me a hint how you would solve my case with a custom ValueFormatter (i.e. extracting multiple attributes from a nested dictionary)? From the code I assume I could only use a single attribute from the nested dictionary.

wvteijlingen commented 8 years ago

You need to create a custom Attribute and a matching ValueFormatter.

class StreamData: NSObject {
    var hdURL: String
    var sdURL: String
    var hlsURL: String

    init(hdURL: String, sdURL: String, hlsURL: String) {
        self.hdURL = hdURL
        self.sdURL = sdURL
        self.hlsURL = hlsURL
    }
}

public class StreamAttribute: Attribute {}

struct StreamValueFormatter: ValueFormatter {
    func unformat(value: NSDictionary, attribute: StreamAttribute) -> AnyObject {
        return StreamData(
            hdURL: value["hd-url"] as! String,
            sdURL: value["sd-url"] as! String,
            hlsURL: value["hls-url"] as! String
        )
    }

    func format(value: StreamData, attribute: StreamAttribute) -> AnyObject {
        return ["hd-url": value.hd-url, "sd-url": value.sdURL, "hls-url": value.hlsURL]
    }
}

Then to your video resource, add a field with the type StreamAttribute: "stream": StreamAttribute(). I've used strings here in StreamData, but you can also use NSURLs, just change the StreamValueFormatter to create NSURL objects.

invliD commented 8 years ago

This is how I implemented it: https://github.com/openHPI/xikolo-ios/blob/b87a563601254d180540ef79691896ed65b297e8/xikolo-common/Data/Model/Video.swift#L57 (ignore the CoreData stuff)

I had to write a lot of code to do what I could do in 6 lines with this PR. Do you still think there shouldn't be support for key paths, @wvteijlingen?

wvteijlingen commented 8 years ago

I think supporting key paths opens up a lot of edge cases. For example, can you filter or sort on nested attributes directly? How would such a filter be represented in the URL? The spec forbids the use of a dot in member names.

You don't have to write an initialiser that takes a dict like you did. You can assign these values directly in the formatter.

invliD commented 8 years ago

Filter is not specified at all and the sorting section contains the following notes:

Note: Although recommended, sort fields do not necessarily need to correspond to resource attribute and association names. Note: It is recommended that dot-separated (U+002E FULL-STOP, “.”) sort fields be used to request sorting based upon relationship attributes. For example, a sort field of author.name could be used to request that the primary data be sorted based upon the name attribute of the author relationship.

The specs allow sorting on key paths: The first note explicitly allows sorting on non-attribute keys and the second one is just a recommendation, but their use case is very similar to the nested object one anyway.

I could have moved the dict-expansion directly into the formatter, but that doesn't really make a difference. The code is necessary, it's just a question of where to put it and I liked this better.

I'm still in favor of supporting key paths, but I agree it might open edge cases that haven't come up yet. I could also imagine a different solution (any ideas?), but the current way with custom attributes, nested model types and value formatters is A LOT of code for a very simple use case, imho.

invliD commented 8 years ago

@wvteijlingen Any opinion?

wvteijlingen commented 8 years ago

I understand your use case and arguments, and I must say I like the simplicity of being able to specify key paths. However, I prefer that Spine resources match the API resources as close as possible. This means using nested objects for nested structures. I worry that if we start adding features like this, we're diverging from that too much. So I'm sorry, but for now I won't merge this.