freshOS / ws-deprecated

⚠️ Deprecated - (in favour of Networking) :cloud: Elegantly connect to a JSON api. (Alamofire + Promises + JSON Parsing)
MIT License
353 stars 32 forks source link

Support keypath in typed calls #17

Closed maxkonovalov closed 8 years ago

maxkonovalov commented 8 years ago

Hey @s4cha! What do you think of adding a resource keypath in ws calls? It's similar to what is implemented in RestKit objects mapping.

Consider the following server response:

{
     count: 2,
     articles: [
          {
               id: 1,
               name: "Foo"
          },
          {
               id: 2,
               name: "Bar"
          }
     ],
     error: 
          { 
               code: 0,
               message: "Test" 
          }
}

It would be great to be able to parse it like this:

ws.get("/articles", params: ["category": name], keypath: "articles").then { articles in 
    // articles = ["Foo", "Bar"]
}.onError { error in 
    // error = "Test"
}

I am aware of the jsonParsingSingleResourceKey and jsonParsingColletionKey, they seem to fulfil the need partially, but if the key is different for different calls, it requires additional setup to be made. As for response error handling, its mapping description could be added globally to WS class similar to jsonParsingSingleResourceKey, specifying the keypath and maybe a custom error class or something like this to be able to parse the specific error format returned by server.

Currently it requires to write a wrapper function like this:

func get<T: ArrowParsable>(path: String, params: [String: AnyObject] = [:], keypath: String? = nil) -> Promise<T> {
    return Promise<T>(callback: { (resolve, reject) in
        ws.get(path, params: params)
            .then({ (json: JSON) in
                if let error = NSError(json: json) {
                    reject(error)
                } else {
                    resolve(self.modelFromJSON(json, key: keypath))
                }
            })
            .onError({ (error) in
                reject(error)
            })
    })
}
s4cha commented 8 years ago

@maxkonovalov This seems like a very good idea !

On top of this, the fact that keypath is optional would make it transparent to update without breaking the api.

I think we're on the same page here,

What I personally strive for is an api that is as simple as possible for 95% of usages but that should still be flexible for specific use cases, without ruining the dev experience for the rest of the 95% :)

On a side note, I would like to sincerely thank you for all the effort you put in contributing to theses libraries. I just created an organisation for those repos, so that they live on their own without being attached to my own account. Indeed I think they are part of something greater, and the organisation will be there to support this vision, the one that has been at the root of these projects from day one Simple tools to make iOS dev a breeze

I know form you contributions that you're totally in line with that and as always, I would love to know your thoughts

Thanks a ton,

Sacha

maxkonovalov commented 8 years ago

Hey @s4cha! I completely agree with your idea to keep the API as simple and as beautiful as possible 😃 But supporting a lot of functionality under the hood is also a must, and Swift helps here to keep the both sides. As you've noticed making the keypath optional with default value being nil should do the work. I'm not sure though if the current jsonParsingSingleResourceKey and jsonParsingColletionKey vars should be kept, I think if the keypath is provided on the call side, it should cover all use cases anyway. But for error handling it would be good to have it the same across the service, parsing it to WSError's jsonPayload. I'd suggest that you start a branch for this and I'll try to contribute to it what I can.

Btw thanks again for the great tools you created and for your trust! Moving it all to an organisation is a good idea, as it will be easier to maintain and evolve all the code. I'd keep all big changes in PR-format though, to be able to discuss and improve them right away.

Sincerely, Max

maxkonovalov commented 8 years ago

Btw, I believe that moving the repositories to the team should break the links in Carthage/CocoaPods because their URLs are now changed. Also it looks like the organisation is private, could you please check this?

s4cha commented 8 years ago

@maxkonovalov Concerning the URLs github should take care of redirecting traffic, is carthage broken on your hand? Seems to work on my computer. Concerning the organisation, it is visible form a private browser but the members were indeed private. You should be able to make yourself visible on https://github.com/freshOS on people tab

maxkonovalov commented 8 years ago

Actually I recently switched over to git submodules for dependencies management, it seems more flexible to me. I just wanted to make sure that it didn't break anywhere, but indeed, seems like github handles all redirects, so no worries))

s4cha commented 8 years ago

Awesome, I'm still going to replace the links in the readmes and pod specs for the future since I don't know how long github will keep redirecting :)

s4cha commented 8 years ago

@maxkonovalov i just created a slack team to be able to discuss all the issues/novelties in a less formal way. Send me your email address @ sachadso@gmail.com and I'll add you ;)

maxkonovalov commented 8 years ago

Ok sure, will send right now :)