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

Query for updating resource #93

Closed markst closed 7 years ago

markst commented 8 years ago

Is it possible to use a Query for updating a resource? We use one type of resource with multiple endpoints.

I'm using a subclassed Router. However there's no real way of identifying which type of request we wish to achieve within URLForResourceType as only ResourceType is passed.

markst commented 8 years ago

URLForQuery constructs a url with the given resource url. I don't see why it shouldn't be possible to construct a url from a resource.URL for POST request as well as a PATCH

wvteijlingen commented 8 years ago

Could you explain your use case for this? Also, what do you mean with "I don't see why it shouldn't be possible to construct a url from a resource.URL for POST request as well as a PATCH"?

markst commented 8 years ago

Hey @wvteijlingen thanks for response.

Here's some logic from Operation.


private func updateResource() {
    let URL: NSURL
    let method: String
    let options: SerializationOptions

    if isNewResource {
        URL = router.URLForResourceType(resource.resourceType)
        method = "POST"
        options = [.IncludeToOne, .IncludeToMany]
    } else {
        URL = router.URLForQuery(Query(resource: resource))
        method = "PATCH"
        options = [.IncludeID]
    }

    return UIColor(hexString: "#2e3a47")!

If I'm updating an object the URL is generated from routers URLForQuery which constructs the URL either using the query's url (resource url) or by the routers URLForResourceType. This is great for being able to specify the endpoint we wish to update the user to, i.e:

updateUser.username = self.usernameView.text
updateUser.URL = NSURL(string: "api/v1/my/user")
SpineSingleton.spine.save(updateUser).onSuccess { (user) in

However if the request is a new resource the above logic uses the routers URLForResourceType to construct the url, which disregards the resource URL.

We wish to use different endpoint for different POST requests. As an example currently if we are signing up with email:

let user = User()
user.email = self.emailValidationView.text
user.password = self.passwordValidationView.text
user.URL = NSURL(string: "api/v1/registrations")
SpineSingleton.spine.save(user).onSuccess { (responseUser) in

or a second option signing up with Facebook auth token:

let user = User()
user.accessToken = token
user.URL = NSURL(string: "api/v1/registrations/facebook")
SpineSingleton.spine.save(user).onSuccess { (user) in

Both return a serialized users object response.

markst commented 8 years ago

This is a little patch to temporarily I'm using currently:

if isNewResource {
    if let URLString = resource.URL?.absoluteString {
        URL = NSURL(string: URLString, relativeToURL: router.baseURL)!
    } else {
        URL = router.URLForResourceType(resource.resourceType)
    }
    method = "POST"
    options = [.IncludeToOne, .IncludeToMany, .OmitNullValues]
} else {
    URL = router.URLForQuery(Query(resource: resource))
    method = "PATCH"
    options = [.IncludeID, .OmitNullValues]
}
wvteijlingen commented 8 years ago

Hmm I see. The problem here is that a Resource's URL property contains the identifying URL for that specific resource instance. So setting user.URL = NSURL(string: "api/v1/registrations") won't work, since that URL is not the URL of that specific user itself.

The cleanest way to solve this would be to create a Registration resource. We'll need to add better customisation options in the Router for building POST URLs. You can then override those and return api/v1/registrations/facebook where needed.

markst commented 8 years ago

@wvteijlingen I did attempt to create a Registration resource the issue I then had was the response resource type matches the saved resource type generic. Which isn't so bad if Registration subclasses User. But then it proved to be more complex when updating the user's attributes & saving.

markst commented 8 years ago

@wvteijlingen what if I were to introduce URLForResource as an alternative or replacement for URLForResourceType in the router?

wvteijlingen commented 8 years ago

I think we can add a URLForResource method that returns either the URL property of the given resource if it exists, or uses URLForResourceType to construct a url otherwise.

markst commented 8 years ago

@wvteijlingen otherwise out of curiosity if I were to go with creating a Registration resource, would it be possible for the callback object to be of the matching type of network request response?:

let registration = Registration()
registration.email = self.emailValidationView.text
registration.password = self.passwordValidationView.text
SpineSingleton.spine.save(registration).onSuccess { (user) in
    if user is User {
        print("Success creating user with id",user.id)
    }

It is my understanding that given the use of BrightFutures, the callback should match the provided generic?

Would it be valid to assume removing the generic would allow for returning different object type?

public func save(resource: Resource) -> Future<Resource, SpineError> {
    let promise = Promise<Resource, SpineError>()
markst commented 8 years ago

Currently it seems that the assumption is the response is always of the type the save is.

Given my above example of saving a Resource of type Registration, then returns a network response with an object of type users. The ResourceFactory correctly instantiates an object of the network response object type, but then the following fails:

SpineSingleton.spine.save(registration).onSuccess { (response) in
    if response is User {
        print("response is User")
    }
    if response is Registration {
        print("response is Registration")
    }
markst commented 8 years ago

Just FYI. I've made modifications to Operation Failable to allow passing the deserialized resource as the success result & Spine's save() to also pass this to the Promise completion. Let me know if this is something you think should be as part of Spine as an option & I'll create a PR.

wvteijlingen commented 8 years ago

Older versions of Spine didn't use generics for returned resources. This allowed the response resource type to differ from the request type. But without the generics, I had to cast every response resource to the correct class. This cluttered the code a lot and it was error prone because it kinda bypasses the Swift type system. So while I get your specific use case I don't feel like removing these generics any time soon.

markst commented 8 years ago

Understood @wvteijlingen. Perhaps introducing an additional save function which doesn't use generics could be possible. Either that or the ability to use our own operations within our project as mentioned #96 could be a solution. Seems a shame to maintain our own fork to enable us to resolve this issue.

wvteijlingen commented 8 years ago

Unfortunately the JSON:API spec is not entirely clear on whether the response type must be the same as the request type. I'm assuming though that it should be, and that this specific case will not happen often. I think a better way to model your API would be to return a Registration that contains a link to the created user.

I don't think this small case justifies extending the API with another save method, sorry.