Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for java libraries directory in the menu #713

Closed
emilianobonassi opened this issue Nov 10, 2015 · 54 comments
Closed

Support for java libraries directory in the menu #713

emilianobonassi opened this issue Nov 10, 2015 · 54 comments

Comments

@emilianobonassi
Copy link

It will be nice to select options for code generation, e.g select custom http libraries for java code.
Do you have plans to add this feature?

PS: probably a duplicate of #712

@cbornet
Copy link

cbornet commented Nov 12, 2015

swagger-editor uses http://generator.swagger.io to generate libs and it is possible to specify the library to use with the "options" field in the request body so it should not be much work to add all java libraries to the editor.
I think this is also a pre-requisite before sunsetting the android generator (see swagger-api/swagger-codegen#1353)

@wing328
Copy link
Contributor

wing328 commented Nov 12, 2015

Yes, that's a pre-requisite.

When sunsetting the Android generator, I suggest we update the name of "Android" to something like

"Android (decommissioned, replaced by Java)"

so that users using Swagger Editor will have a better idea about the change.

@cbornet
Copy link

cbornet commented Nov 12, 2015

Arf ! Not so easy : swagger-editor gets the client list from http://generator.swagger.io/api/gen/clients which doesn't give information about supported libraries so the generator must be modified first.

@cbornet
Copy link

cbornet commented Nov 12, 2015

We can get the supported libs from https://generator.swagger.io/api/gen/clients/java but that will make a lot of requests if it needs to be done for each language. So maybe it would be better to have a resource that outputs all languages with all libraries all at once.

@wing328
Copy link
Contributor

wing328 commented Nov 12, 2015

What about retrieving the option only when someone clicks on the programming language in the dropdown menu? (For example, if I want to generate a Ruby API client, whether Java library supports 5 different HTTP libraries does not interest me)

@cbornet
Copy link

cbornet commented Nov 12, 2015

Yes. I don't know Angular very much. Is it possible to dynamically open a sub-menu with the result of the request ?

@wing328
Copy link
Contributor

wing328 commented Nov 12, 2015

Well I'm no expert in Angular either. I'm just making suggestions from the user's perspective :)

@cbornet
Copy link

cbornet commented Nov 12, 2015

A quick google search seems to show that it is possible so this looks like the way to go 😄

@emilianobonassi
Copy link
Author

I've not read the swagger-editor code but @cbornet let me point the issue.
First we have to modify swagger gen to output all possible options for all clients/server lib.
Then parse it client-side and display on the menu.

My idea, from user perspective, is to display a submenu for each language which supports additional libraries, i.e. Generate Client -> Java -> [retrofit, retrofit2 etc..](where [] is a dropdown list).

I've developed a project in AngularJS so I'm confident I can implement that feature.

I prefer we decide how we split the work and then I will start.
Is it ok for you, @cbornet and @wing328 ?

@cbornet
Copy link

cbornet commented Nov 12, 2015

@emilianobonassi the codegen already has a resource for displaying options : see https://generator.swagger.io/api/gen/clients/java for instance. So this is just editor implementation.
Given my experience with Angular, I'll gladly let you implement this 😄

@emilianobonassi
Copy link
Author

@cbornet , yes, I referred to your suggestion:

We can get the supported libs from https://generator.swagger.io/api/gen/clients/java but that will make a lot of requests if it needs to be done for each language. So maybe it would be better to have a resource that outputs all languages with all libraries all at once.

As you stated, it's more efficient (and backward compatible) to optimize the endpoint so it will returns a json with all possible options for every client, may be something like "https://generator.swagger.io/api/gen/clients?with-all-possible-options=true". What do you think?

If you want we can implement both, Angular is a beautiful and useful framework to learn. I'm not the top on Angular but I can do.

@cbornet
Copy link

cbornet commented Nov 12, 2015

It is not possible to have different response models for a given swagger operation so that would mean a breaking change in the generator API which I think is unwanted. A new path resource would be preferable I guess.
As @wing328 stated, the ideal would be to make the request only when the user selects a given lib : when you click on "Java", it sends the request and if the response has libraries, it opens a submenu to select the library. Do you think this is possible ?

@emilianobonassi
Copy link
Author

@cbornet , both options are possible.

I've just implemented the second option, get a look (click on java) and give me a feedback.
http://jsfiddle.net/7c49zt5h/2/

@cbornet
Copy link

cbornet commented Nov 12, 2015

Seems nice 👍

@emilianobonassi
Copy link
Author

Anyway from UX perspective in my opinion the best option is a new endpoint with full data otherwise how can you inform the user that options are available for a client/server?

@cbornet
Copy link

cbornet commented Nov 12, 2015

He will first chose his language, then its library but I can understand your point.
@wing328 what is your opinion ?

@emilianobonassi
Copy link
Author

Anyway, another idea is to the user the option to insert custom entries in the menu, specifying the language and custom options. By this way, we can modify swagger-editor to fill the menus with data from swagger-generator and from the user custom data. The latter may be pre-filled with the customization we are talking about. What's your idea?

@cbornet
Copy link

cbornet commented Nov 12, 2015

I find it less user friendly if you just want to select a lib

@emilianobonassi
Copy link
Author

Thank you very much for your feedback.

I vote for the new endpoint and to fill the menu with submenus on the client/server which admits them. It doesn't change substantially the UI, it only adds a lateral menu for those libraries and it appears only on hover (instead of click as in the example). The solution is in the middle between a request for each client and a request only on click but offers a better UX, in my opinion.

Waiting for @wing328 , if he won't reply I will prepare anyway a PoC in my forks (and send you the link). At the end, he will reply when I will do the PR.

@wing328
Copy link
Contributor

wing328 commented Nov 13, 2015

Not sure if the size of the JSON response that contains all the options returned by the new endpoint would be a concern given that it will return a lot more than just a list of programming languages supported by the generator (http://generator.swagger.io/api/gen/clients returns 493b only).

Since you're the one who's making the change, we can go with your approach first and revise accordingly later based on user's feedback.

@cbornet
Copy link

cbornet commented Nov 13, 2015

@emilianobonassi Also note that your approach takes longer to implement since we need to first update generator.swagger.io with the new endpoint.
@wing328 I can do the new endpoint if we go that way. Do we expose all options or just the libraries name if any ? What should be the def for it ?

GET /gen/{servers or clients}-extended

[
{"language": "java",
 "description": "Generates a Java client library",
  "options":{
  "sortParamsByRequiredFlag": {
    "opt": "sortParamsByRequiredFlag",
    "description": "Sort method arguments to place required parameters before optional parameters.",
    "type": "string",
    "default": "true"
  },
  "ensureUniqueParams": {
    "opt": "ensureUniqueParams",
    "description": "Whether to ensure parameter names are unique in an operation (rename parameters that are not). Default: true",
    "type": "string"
  },
  "modelPackage": {
    "opt": "modelPackage",
    "description": "package for generated models",
    "type": "string"
  },
  "apiPackage": {
    "opt": "apiPackage",
    "description": "package for generated api classes",
    "type": "string"
  },
  "invokerPackage": {
    "opt": "invokerPackage",
    "description": "root package for generated code",
    "type": "string"
  },
  "groupId": {
    "opt": "groupId",
    "description": "groupId in generated pom.xml",
    "type": "string"
  },
  "artifactId": {
    "opt": "artifactId",
    "description": "artifactId in generated pom.xml",
    "type": "string"
  },
  "artifactVersion": {
    "opt": "artifactVersion",
    "description": "artifact version in generated pom.xml",
    "type": "string"
  },
  "sourceFolder": {
    "opt": "sourceFolder",
    "description": "source folder for generated code",
    "type": "string"
  },
  "localVariablePrefix": {
    "opt": "localVariablePrefix",
    "description": "prefix for generated code members and local variables",
    "type": "string"
  },
  "serializableModel": {
    "opt": "serializableModel",
    "description": "boolean - toggle \"implements Serializable\" for generated models",
    "type": "string"
  },
  "fullJavaUtil": {
    "opt": "fullJavaUtil",
    "description": "whether to use fully qualified name for classes under java.util",
    "type": "string",
    "default": "false"
  },
  "library": {
    "opt": "library",
    "description": "library template (sub-template) to use",
    "type": "string",
    "enum": {
      "<default>": "HTTP client: Jersey client 1.18. JSON processing: Jackson 2.4.2",
      "jersey2": "HTTP client: Jersey client 2.6",
      "okhttp-gson": "HTTP client: OkHttp 2.4.0. JSON processing: Gson 2.3.1",
      "retrofit": "HTTP client: OkHttp 2.4.0. JSON processing: Gson 2.3.1 (Retrofit 1.9.0)"
    },
    "default": "<default>"
  }
},
{ "language" : ...
}
]

or just

[
{"language": "java",
 "libraries": ["<default>","jersey2", "okhttp-gson","retrofit"]
},
{"language": "android",
 "libraries": []
},
...
]

@wing328
Copy link
Contributor

wing328 commented Nov 13, 2015

I prefer exposing all options so that the user is aware of different options available to customize the SDK.

For the format/def of the response data, I do not have a strong opinion on it as long as the data can be easily parsed by the front end (Swagger Editor in this case).

@cbornet
Copy link

cbornet commented Nov 13, 2015

OK
@emilianobonassi Will you be able to parse the first format easily ?

@emilianobonassi
Copy link
Author

Thanks for the answer.
In order

  • @wing328

    Not sure if the size of the JSON response that contains all the options returned by the new endpoint would be a concern

    json size is not a problem

    Since you're the one who's making the change, we can go with your approach first and revise accordingly later based on user's feedback.

    perfect.

  • @cbornet

    I can do the new endpoint if we go that way
    Thank you!

    Will you be able to parse the first format easily ?
    Yes, it is native in javascript. I would start to parse only the library option for each client/server, then (in another issue I think) we may extend the topic on other options. May be when you click on a specific library you may see a dialog with checkboxes and other ui object to grab user willings. What do you think?

@cbornet , at the end, let me know when the new endpoint will be ready.

@webron
Copy link
Contributor

webron commented Nov 13, 2015

Thanks for the input everyone.

The exposed options is actually a relatively new feature and we are looking into how to integrate it with the editor. We appreciate your feedback and suggestions, which we'd take into account, but for now we're going to keep the current API of the online generator as-is.

@cbornet
Copy link

cbornet commented Nov 14, 2015

@webron OK, this is understandable. Thanks for your input.
@emilianobonassi That leaves us for now with the request at language selection. Still OK to do it ?

@emilianobonassi
Copy link
Author

@webron I understand you perfectly, we don't modify (as now) the API. What do you think about the idea to display a dialog with respective options when the user click on a specific client/server? By this way we don't modify the API and retrieve client options when the user clicks.

@cbornet , for me it is ok to do it but what do you think about the latter idea I've just told to @webron ?

@emilianobonassi
Copy link
Author

I will post a jsfiddle/codepen on monday with a sketch of the idea.

@psychowood
Copy link

My 2 cents: I'd suggest also the ability to store/read the options directly in the json/yaml itself too, e.g. in a x-codegen-options attribute.

@cbornet
Copy link

cbornet commented Nov 30, 2015

@emilianobonassi well, the primary idea for this issue was the ability to select the library which IMO shouldn't be an "expert" config (on the contrary to the other options). I understand the idea behind your dev but I think this should go in another issue.

@emilianobonassi
Copy link
Author

@psychowood , store/read options in swagger-editor json config file is it ok for me but not in the APIs description, it should be independent from that, shouldn't it?

@cbornet , I agree with you that the issue may be splitted in two ones. Anyway I've have some difficulties to think about the UI. Do we remain with http://jsfiddle.net/7c49zt5h/2/ ?

@saharj
Copy link
Contributor

saharj commented Jan 20, 2016

What do you think about this @fehguy? Are we adding this to codegen?

@psychowood
Copy link

@emilianobonassi, sure, as long as it does follows swagger specs.
E.g. https://apimatic.io/blog/post/swagger-2-0-extension-for-code-generation-settings

@emilianobonassi
Copy link
Author

@psychowood , very interesting idea, this may be the way!

@saharj saharj added this to the Backlog milestone Feb 6, 2016
@cbornet
Copy link

cbornet commented Jun 27, 2016

A lot of people are afraid of using libraries because they are not selectable in the editor. Can we do something about this issue ?

@emilianobonassi
Copy link
Author

@cbornet , thanks for resume this issue. As you stated, now a bunch of people are calling for it, we can follow the route indicated by @psychowood but I'd like to involve @webron on the discussion to agree on the direction. Waiting for him to start!

@webron
Copy link
Contributor

webron commented Jul 7, 2016

Sorry for the delay on this, thank you for your patience. Please feel free to go ahead with it if you wish.
One thing though - the thread is a bit long and I'm not sure what changes/proposals are relevant (both for the editor and the codgen) - can you point me to the relevant parts?

@cbornet
Copy link

cbornet commented Jul 7, 2016

Yes there are a lot of ideas and I think most of them shoumd go in other issues. The main and most wanted feature of this issue is to have the ability to choose the library like this.

@webron
Copy link
Contributor

webron commented Jul 7, 2016

@cbornet - you mean the ability to pick the underlying implementation under a specific template (java in the linked example)? Sounds good to me.

@cbornet
Copy link

cbornet commented Jul 8, 2016

Yes, exactly that.

@emilianobonassi
Copy link
Author

@webron, no problem for the delay.

We can start from the PoC @cbornet linked that I did, than in the future we may extend it with specific options.

I am going to develop that feature for every language that admits a specific template. I will post a jsfiddle before integrating it in the project.

Do you agree with me?

@webron
Copy link
Contributor

webron commented Jul 8, 2016

Sounds good.

@emilianobonassi
Copy link
Author

Great! Thanks @webron !

@szantopeter
Copy link

FYI: I made a pull request to solve the same issue : swagger-api/swagger-codegen#3516 my solution is rather a workaround, still it worth mentioning.

@cbornet
Copy link

cbornet commented Oct 14, 2016

@emilianobonassi are you on it ?

@emilianobonassi
Copy link
Author

@cbornet thanks for the follow-up.

Sincerely, not actually. I did some work but I stopped. In this week, I am going to understand if I have time or leave the ball. I am sorry.

@emilianobonassi
Copy link
Author

Guys, I am really sorry but I've not time to invest in this project. I hope to have it in the near future.

@szantopeter FYI this can be a starting point,http://jsfiddle.net/7c49zt5h/6/ (click on Java or Android for example).

@cbornet
Copy link

cbornet commented Jun 8, 2017

Could someone take this ? I really think it's an important/wanted feature.

@ponelat
Copy link
Member

ponelat commented Sep 16, 2021

Hey folks, closing this out as there doesn't appear to be any appetite in adding user interface for configuring Codegen options from SwaggerEditor. At least not since 4 years ago.

The workaround is to download the definition and use the SwaggerCodegen directly, with it's options. A starting point can be found here: https://swagger.io/docs/open-source-tools/swagger-codegen/.

If anyone would like to revive this issue and has some appetite, feel free to re-open.

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

No branches or pull requests

8 participants