Skip to content
This repository has been archived by the owner on Jan 20, 2024. It is now read-only.

Chaining and the Naming Problem. #9

Open
sichvoge opened this issue Feb 5, 2015 · 6 comments
Open

Chaining and the Naming Problem. #9

sichvoge opened this issue Feb 5, 2015 · 6 comments

Comments

@sichvoge
Copy link

sichvoge commented Feb 5, 2015

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
Copy link
Author

sichvoge commented Feb 5, 2015

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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

@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
Copy link

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
Copy link

orubel commented Mar 21, 2017

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants