Skip to content

[REQ][CSHARP][GENERICHOST] Support stream for binary response#21677

Merged
wing328 merged 7 commits intoOpenAPITools:masterfrom
alec-petersen:support-stream-on-generichost
Aug 11, 2025
Merged

[REQ][CSHARP][GENERICHOST] Support stream for binary response#21677
wing328 merged 7 commits intoOpenAPITools:masterfrom
alec-petersen:support-stream-on-generichost

Conversation

@alec-petersen
Copy link
Copy Markdown
Contributor

@alec-petersen alec-petersen commented Aug 1, 2025

Fixes #21676

Creates a switch statement on the HttpStatusCode to determine how to create the ApiResponse.

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 || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in WSL)
    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*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR solves a reported issue, reference it using GitHub's linking syntax (e.g., having "fixes #123" present in the PR description)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@devhl-labs
Copy link
Copy Markdown
Contributor

Very cool, thanks! Can you add something to the petstore and build the samples?

@alec-petersen
Copy link
Copy Markdown
Contributor Author

@devhl-labs Thank you! Looks like I got some work to do for backwards compatibility.

@alec-petersen
Copy link
Copy Markdown
Contributor Author

@alec-petersen alec-petersen force-pushed the support-stream-on-generichost branch from f08f62d to 58259f2 Compare August 2, 2025 15:59
@wing328
Copy link
Copy Markdown
Member

wing328 commented Aug 10, 2025

@alec-petersen thanks for the pr

@devhl-labs is this PR ready? if yes, i will get it merged asap

{{/responses}}
{{#responses}}
{{#isBinary}}
{{#-first}}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you want the first response which is a binary, but i don't think this first will filter the way you want.

Copy link
Copy Markdown
Contributor Author

@alec-petersen alec-petersen Aug 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do ya mean? And yeah the goal here is just to add the section if there is a binary response, there may be a better way of accomplishing that in a more mustachey way haha

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the first will get rendered every time. Pretty sure we can only use first like this on arrays.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a first lambda besides the one you used here but it requires a two space delimiter which is not ideal for a multiline string.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to have the effect I intended, I'd need to understand the failure scenario maybe.

@devhl-labs
Copy link
Copy Markdown
Contributor

Lets just send it. My concern can be fixed in a another pr.

@wing328 wing328 merged commit 4b88cf8 into OpenAPITools:master Aug 11, 2025
58 checks passed
@alec-petersen alec-petersen deleted the support-stream-on-generichost branch August 22, 2025 19:49
Goopher pushed a commit to Goopher/openapi-generator that referenced this pull request Sep 9, 2025
…ITools#21677)

* Add ContentStream property to ApiResponse

* Add a constructor which sets ContentStream

* Create and use a memory stream for binary model types

* Remove extra space

* Update samples

* Add {{nrt?}} to support older versions

* Remove public modifier for interface property
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.

[REQ][CSHARP][GENERICHOST] Support stream for binary response

3 participants