OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
20.54k stars 6.27k forks source link

[cpp-qt-client] Qt progress info #18950

Open Jazzco opened 1 week ago

Jazzco commented 1 week ago

When downloading larger files via the generated API I miss a possibility to get the download progress. This could be achieved by connecting to the signal QNetworkReply::downloadProgress. This is solution 1 of the request: https://github.com/OpenAPITools/openapi-generator/issues/18926

PR checklist

Jazzco commented 1 week ago

Remark for step 3: I work on Windows where all touched files get the CR/LF which results in too many changed files. So please take over the changes.

FYI: @ravinikam (2017/07) @stkrwork (2017/07) @etherealjoy (2018/02) @martindelille (2018/03) @muttleyxd (2019/08)

wing328 commented 1 week ago

thanks for the PR. I've filed https://github.com/OpenAPITools/openapi-generator/pull/18956 with updated samples (some tabs are replaced with 4-space)

but some CI tests failed: https://github.com/OpenAPITools/openapi-generator/actions/runs/9558830005/job/26348400169?pr=18956

Does this change work with Qt 5.x?

Jazzco commented 1 week ago

Does this change work with Qt 5.x?

Would be mystic if not. You already have similar code in api-body:677

Jazzco commented 1 week ago

I ran step 3 from a linux vm to complete this.

Jazzco commented 1 week ago

I tested the changes in a local old build of our source. It builds without errors using these changes under Qt 5.15.2 on Windows platform.

Jazzco commented 1 week ago

I'm not quite sure this works correctly:

As I'm not the mustache specialist what do you think?
@ravinikam (2017/07) @stkrwork (2017/07) @etherealjoy (2018/02) @MartinDelille (2018/03) @muttleyxd (2019/08)

Jazzco commented 1 week ago

I didn't test it but it makes sense!

I added the downloadProgress to the parts where the request-finished is connected. But it looks a bit odd in the generated code of the examples. According to the names of the pet examples it seems to make sense to add the progress info only to uploadFile. And as there are up- and download I would name it {{nickname}}Progress.

  1. Is there a possibility to add a function only when a specific flag is set, so the user can decide in the interface if the progress should be available?

  2. It looks like it would be sufficient to only connect to worker and there is no need to additionally connect to _latestWorker. If that's correct I would remove the latter connects.

Jazzco commented 1 week ago

I changed this PR to draft until this is discussed.

wing328 commented 1 week ago

Is there a possibility to add a function only when a specific flag is set, so the user can decide in the interface if the progress should be available?

yes, you can add an option to do so. https://github.com/OpenAPITools/openapi-generator/pull/18072 (for C client) is a good reference PR as a starting point.

Jazzco commented 1 week ago

yes, you can add an option to do so. #18072 (for C client) is a good reference PR as a starting point.

That's a huge change, can you please point me to that option? What's its name?

wing328 commented 1 week ago

https://github.com/OpenAPITools/openapi-generator/pull/18072/files#diff-650ea35dcd5ff1dee971a524e6c1e7f1d538319357fa079c54e0a9b949fbdb92R313 to add an cli option

https://github.com/OpenAPITools/openapi-generator/pull/18072/files#diff-650ea35dcd5ff1dee971a524e6c1e7f1d538319357fa079c54e0a9b949fbdb92R325 to process it in additional properties

https://github.com/OpenAPITools/openapi-generator/pull/18072/files#diff-e5916826f6d19371fe33e8a59d2bf7a825844bc77253255ddbe69ae8c56d5cedR323 to use it in the template

to make your life easier, can you please wrap the new code block for Qt progress info in the template with {{#addDownloadProgress}} ... {{/addDownloadProgress}} ?

i'll then take care of the rest

Jazzco commented 1 week ago

This should be right now: