Chaining and the Naming Problem.

sichvoge commented 9 years ago

Out of the conversation between @blakeembrey and me:

Christian Vogel Hi Blake, do you also store the parents?

Blake Embrey parents? the raml client generator? yes

Christian Vogel yes sorry the client gen ^^ - as your snippet is iterating through the children and checks if there is an uriparameter - in my example I would need to iterate through the parents to find out the parameter for an operation

Blake Embrey hmm, i'm not sure on the use case but i can make sure it's there

Blake Embrey yes

Blake Embrey https://github.com/mulesoft/raml-client-generator/blob/master/lib/compile/context/resources.js#L122

GitHub mulesoft/raml-client-generator raml-client-generator - Template-driven generator of clients for APIs described by a RAML spec

Christian Vogel it's really simple - let's assume you have the following /sample/{sampleId}/status

Christian Vogel that has to generate void getStatus(String sampleId)

Christian Vogel more complex could be /lab/{labId}/sample/{sampleId}/status/ -> void getStatus(String labId, String sampleId)

Christian Vogel you know what I mean? (edited)

Blake Embrey i can see it, but that's a much different dsl to the current implementation

Blake Embrey how are you generating the names of methods?

Blake Embrey you're using the param ids instead of names right?

Christian Vogel generating the method could be

{{#each allResources}}
{{#each methods}}
void {{key}}{{../key}}(???)
{{/each}}
{{/each}}

Blake Embrey but you'll have easily conflicting names, that's the main reason behind the verbose chained api that exists in the javascript client

Christian Vogel that is true, but no one wants to have method names like resource1, resource2, etc.

Christian Vogel in javascript it might work, but not in other languages

Christian Vogel and i think it is very hard to have naming conflicts - as you will always have different parameters no?

Blake Embrey that's not what it's named

Blake Embrey those are placeholders

Blake Embrey the js client works by initialising classes that have unique names

Blake Embrey the methods on each class is the actual uri name

Blake Embrey (the key property)

Blake Embrey that's how i achieved efficient chaining in the javascript dsl

Christian Vogel if you chain it might work - but i am talking about non chaining methods

Blake Embrey
e.g.

Resource5.prototype.status = function () {
  return new Resource6(this._uri + '/status');
};

Blake Embrey yes, so i was asking why it's non chaining and how you plan to name the methods

Blake Embrey i would love a non-chaining dsl too, but i've thought about it and couldn't find a good solution

Christian Vogel why it shouldn't be chaining - one example in java would be: if you have 5 nested resources - you would have to say Client.Res1.Res2.Res3.Res4.Res5.Res5Response response = ...

Blake Embrey i didn't see the why, sorry

Christian Vogel it is just not best practice

Blake Embrey ok, sure

Blake Embrey what about the solution for naming?

Christian Vogel in javascript it might be more common as you have var keyword, but when you work with objects and you have to chain - you might end up with with a non readable code. this is why we have OO right. If we wouldn't - we would just have one class and everything is nested in this class.

Christian Vogel how we solve the naming issue - good question.

Blake Embrey the argument against the chaining doesn't really make sense though since i know it's not great but we should try to be consistent - if we're changing it here we should change the js one too

Blake Embrey it's not so much it's non-readable, it's pretty good since it's based on resource paths

Blake Embrey but it gets too long

Blake Embrey e.g. client.resources.users.userId(55).get() isn't awful, but already a bad start

Blake Embrey it gets worse for longer resources

Christian Vogel it is just that in languages like Java - nested classes are not really common

Blake Embrey neither in js

Blake Embrey in fact, i really never see it used

Christian Vogel can you send me an example of where you would have naming problems

Blake Embrey but i needed it for performance and name spacing

Blake Embrey there's lots, it's a matter of working out how to name it

Blake Embrey e.g. that path above would be get /users/{userId} in raml

Blake Embrey what's it in a single name?

Christian Vogel i haven't seen any nested classes in java as well - why should we start to do it. we should add an issue to github or at least raise it inside the forum when we have the appropriate labels in there

Christian Vogel it would be getUsersByUserId(String userId)

Blake Embrey what about /users/{userId}/photo?

Christian Vogel getPhotoByUserId(String userId)

Blake Embrey ok, now we need to spec this so it works everywhere

Blake Embrey and we implement that logic into the client generator and remove the chaining from the js implementation too

Christian Vogel this one might be wrong and more difficult as you want to have a single user - getUsersByUserId(String userId)

Christian Vogel but this is something we could find out

sichvoge commented 9 years ago

This issue is basically to discuss how we are gonna solve the naming conflict issue. The conversation above shows where we are right now. :)

blakeembrey commented 9 years ago

Ok, so some summarisation:

We need to some how spec a way to translate full paths into method names while staying clear on what the method does. The goal is to have a flat structure that is fully readable in code. E.g. GET /users/{userId} -> getUserByUserId
How do we handle more complex chains like /users/{userId}/photo which will likely want to reorganise like getUserPhotoByUserId?
How do we handle non-nouns? E.g. /search/repositories doesn't really work with getSearchRepositories, we probably want to keep that the same.
How do we map method names? E.g. GET /users -> getUsers, POST /users -> createUser, PUT /users/{userId} -> updateUser
Can we something new to the spec or reuse something like displayName?

justjacksonn commented 9 years ago

Take a look at your jax-rs module for an example of taking RAML and generating Java classes. I think it uses sub-resources and method chaining for paths with more parts. I don't know that it would be ideal, but if you try to solve the naming problem with a path like /a/{a-id}/b/{b-id}/c/{c-id}/d/e/f/{f-id}/g with a single method name.. I can't imagine you're going to have much success. It seems to me the right "algorithm" is some sort of chained setup depending on the language like what you are doing now.

Let's keep this in perspective with what we are building. My thought is.. the SDK generated from RAML is a 1 to 1 rest call. It's not meant to be a perfect getUserPhotosForUserId() method naming process. It's meant to be consumable so that the entire RAML API can be put into SDKs for different languages. Once we have a generated SDK that works.. we can (and probably should) write "helper" or "sample" SDK classes built on top of generated classes that provide the more "shimmed" SDK that we would want users to use. For example, if you have /users/{user-id}/photos/{photo-id}/details as a path, do you want to some how figure out a single perfect SDK generated method called getDetailsForPhotoIdByUserId? Or getUserPhotoDetailsForUserIdAndPhotoId? It's going to be impossible... or at the very least not make method names any prettier the longer/more parts the paths are.

So I'd suggest we build the SDK to use sub-resources and/or chained method calls. I did something similar years ago.. although I used java reflection to build up a get/set method chain based on a form request parameter. Essentially, to auto-populate a nested POJO, we'd put in something like user.name.address.city as the form field name, and the code would create getUser().getName().getAddress().setCity(value). Now this of course was not SDK code that others were going to consume.. so I understand our need to try to make something somewhat nicer than a long chained list of method/function names, but to me more important is that the full SDK be auto-generated and that I can then consume it in my favorite language.

I suspect that Christian is after a more developer friendly generated SDK where by I agree, a getUsersByUserId() method with a few parameters is far more consumable and documentation friendly than a potentially long chained list of calls. However, think about writing a CLI that "eats your own dog food" by consuming your SDK.. you probably don't want to release a CLI to the world with a one to one mapping of resources.. it wouldn't be very easy to use. Instead you may want to shim a CLI with far less calls but more functional and developer friendly. Likewise, for me, I'd want my developers to consume an SDK with methods like getUserPhotoByUserId() over getUser(id).getPhoto(id) (or worse the more path parts there are). So my gut says we should first get a 1 to 1 generated SDK working for different languages, and instruct/teach how to "shim" the more developer friendly version that consumes the generated SDK.

Thoughts?

blakeembrey commented 9 years ago

@justjacksonn I like your take on it and I think it make a lot of sense.

My concern is still that this problem may be easily is fixable by adding a single field to the RAML specification (or taking advantage of "metadata" - x-client-method: getPhotoByUserId - in RAML 1.0). Until that exists, I think the chaining approach is the best and agree that it'd be difficult to auto-generate method names.

Edit: Right now the context logic is generating usable usable method names at every piece of the resource path, so it should be possible to build something in each language to follow it consistently (albeit with stylistic differences - like camelCase vs snake_case).

justjacksonn commented 9 years ago

Interesting option using the 1.0 spec (when is it going to be out?? ;) x-client-method: option. My concern there, and possibly not warranted, is tying in a "sdk generated" name into the API definition. I am not convinced it's a bad idea though.

orubel commented 7 years ago

You have several other issues as well; you have to avoid 'redirects' which can cause additional 'REQUESTS' and changes in I/O pathing which will cause additional REQUEST in distributed architectures.

For example, if you have moved your security to an edge case like Zuul (Netflix's proxy server), you will have to check security for every new API endpoint. Thus, you will:

drop the thread
create a new request
go outsides the DMZ and go to the Proxy to validate
then reenter the server and go to your new endpoint for every link in your chain

This does not improve speed or performance.

What should occur is that you should abstract your communication layer from your biz logic so that communication can be shared without creating a new request and so I/O can be synchronized in a distributed architecture.

mulesoft-labs / raml-client-generator

Chaining and the Naming Problem. #9