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
264 stars 109 forks source link

Restrict fields for included resource #109

Closed kirpichenko closed 7 years ago

kirpichenko commented 7 years ago

Hi,

I stuck at some point. Let's say I download Comment resources. Each comment has it's own Author. I create query like

var query = Query(resourceType: Comment.self, path: "comments")
query.include("author")

Now I want to fetch authors names only. Looks like restrictFieldsOfResourceType(type: to:) is what i need. The description says This method can be used to restrict fields of included resources.

I do

query.restrictFieldsOfResourceType(Author.resourceType, to: "name")

but the first cycle of this method will check if the field name is available for Comment class (not Author)

So the question is it possible to restrict fields for included resource and what is the correct way to do this?

Thanks

wvteijlingen commented 7 years ago

Yes, this is definitely possible and restrictFieldsOfResourceType is the way to do this. I see that there is a bug in the method. It should check the fields of the included type, instead of the main type.

wvteijlingen commented 7 years ago

I pushed an update to master that should fix this.

kirpichenko commented 7 years ago

Thanks. Will check it.

kirpichenko commented 7 years ago

Looks like we still have an issue with that. restrictFieldsOfResourceType adds the field to the correct resource type but now JSONAPIRouter crashes when tries to add filters

// Fields
for (resourceType, fields) in query.fields {
    let keys = fields.map { fieldName in
    return keyFormatter.format(T.fieldNamed(fieldName)!)
    }
    ...
}

resourceType is correct (authors in my case), but it tries to find field in Comment resource (T.fieldNamed(fieldName)!)

wvteijlingen commented 7 years ago

Hmm, this is quite a problem since it makes restrictFieldsOfResourceType completely unusable. Unfortunately the Router doesn't have a way to access the fields of other resource types.

kirpichenko commented 7 years ago

I'm not familiar with the code base but what if to save fields for the Query instances as

var fields: [ResourceType: [Field]]

instead of

var fields: [ResourceType: [String]]

in this case it is not necessary for the router to ask fields of the other resource type

What do you think? Any objections for this?

wvteijlingen commented 7 years ago

That's how it worked at first. The problem is that the field names in Swift do no necessarily match the JSON keys. So the Router uses a KeyFormatter to get the JSON keys from the Swift field names. So we need to store the entire Field for that.

wvteijlingen commented 7 years ago

We might be able to store the serializedName in the query, and then let the KeyFormatter format that to a key, but it requires some refactoring of the key formatting process.

wvteijlingen commented 7 years ago

I updated this in master, so it should work now. Feel free to reopen this issue if it's still broken for you.