Skip to content

[dart][dart-dio] Improved library specific template usage - #10742

Merged
wing328 merged 1 commit into
OpenAPITools:masterfrom
kuhnroyal:dart-dio/templates
Apr 10, 2022
Merged

[dart][dart-dio] Improved library specific template usage#10742
wing328 merged 1 commit into
OpenAPITools:masterfrom
kuhnroyal:dart-dio/templates

Conversation

@kuhnroyal

Copy link
Copy Markdown
Contributor

Improved library specific template usage.
This is just an idea on how to make templating for different libraries a little bit easier.
With this the main templates don't have to be changed when adding a new library.
It could be used for the new json_serializable library option that is happening in some branch.

Any thoughts?
CC @jaumard (2018/09) @josh-burton (2019/12) @amondnet (2019/12) @sbu-WBT (2020/12) @kuhnroyal (2020/12) @agilob (2020/12) @ahmednfwela (2021/08)

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master (5.3.0), 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@ahmednfwela

ahmednfwela commented Oct 31, 2021

Copy link
Copy Markdown
Contributor

what do the samples look like for these ?
won't this line {{#includeLibraryTemplate}}enum{{/includeLibraryTemplate}} just write the string "enum" if includeLibraryTemplate is not null ?

Edit: nvm, didn't notice the changes in DartDioNextClientCodegen.java

@ahmednfwela

Copy link
Copy Markdown
Contributor

this is actually very cool, we can take it a step further to generalize all serialization operations in the api
I believe these are the parts that need to be delegated to separate files

  1. api file input -> just like your api/constructor file, but I think it should be split into three parts
    1. fields
    2. constructor
    3. constructor call -> for use in api_client.mustache file
  2. api input parameters -> generates the _path, _bodyData, _options and _queryParameters that will be given later to the dio request
  3. api results -> receives the proper response from dio (based on the status code) and serialize it to the response type, this should include non-2xx codes, as I believe they should be serialized as well.
  4. model files

one other issue to think about is inline enums, since they are scoped per request, each serialization library has its own handling of it

@kuhnroyal

Copy link
Copy Markdown
Contributor Author

Yeah the samples are up to date :)

I can move this forward and add it in all places if there is a general consensus.

Then we can base the json_serializable option on this.

@kuhnroyal

Copy link
Copy Markdown
Contributor Author

@wing328 what do you think about this approach? I would like to reuse the existing TemplateManager from the generator but didn't find a way to access it.

@agilob

agilob commented Nov 3, 2021

Copy link
Copy Markdown
Contributor

It's a good approach, it would be valuable to see it working on bigger and more complex generators

@kuhnroyal

Copy link
Copy Markdown
Contributor Author

Not sure it is applicable on many generators, I think this approach works well here because we don't have a switch for the http library (dio) so all the other serialization library specific parts are very small and scoped.

@kuhnroyal
kuhnroyal force-pushed the dart-dio/templates branch 2 times, most recently from e2f0057 to c72087d Compare January 9, 2022 18:14
@kuhnroyal
kuhnroyal marked this pull request as ready for review January 9, 2022 18:14
@kuhnroyal

Copy link
Copy Markdown
Contributor Author

@wing328 I think this can be merged

@kuhnroyal

Copy link
Copy Markdown
Contributor Author

Everything else can be adapted to this approach when there is a 2nd serialization library

@wing328

wing328 commented Mar 29, 2022

Copy link
Copy Markdown
Member

@kuhnroyal sorry that I missed your message earlier. Can you please resolve the merge conflicts when you've time?

@kuhnroyal

Copy link
Copy Markdown
Contributor Author

@wing328 rebased

@kuhnroyal
kuhnroyal force-pushed the dart-dio/templates branch from da597a8 to 1dc3929 Compare April 6, 2022 14:54
@wing328 wing328 added this to the 6.0.0 milestone Apr 10, 2022
@wing328
wing328 merged commit 51c4851 into OpenAPITools:master Apr 10, 2022
@kuhnroyal
kuhnroyal deleted the dart-dio/templates branch April 10, 2022 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants