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

[haskell][server] Include response headers in the API type #13565

Merged
merged 1 commit into from Dec 6, 2022

Conversation

7omb
Copy link
Contributor

@7omb 7omb commented Oct 1, 2022

In servant response headers are part of the type of the API. If headers are included in the response object of the OpenAPI spec they are now added to the Haskell type. If no headers are given the type of the Haskell API is unchanged.

Since headers in the OpenAPI spec is a map multiple Set-Cookie headers are currently not possible. If someone uses the workaround with null bytes, the null bytes are removed and each header is headed to the list.

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 (6.1.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@7omb 7omb changed the title Include response headers in the API type [haskell][server] Include response headers in the API type Oct 1, 2022
@7omb 7omb force-pushed the use-response-headers-in-api-type branch 2 times, most recently from 5b4743e to 20d934c Compare October 5, 2022 05:42
@wing328
Copy link
Member

wing328 commented Oct 6, 2022

@7omb thanks for the PR.

This is not a breaking change (non-backward compatible), right?

cc @frol (2017/07) @farcaller (2017/08) @richardwhiuk (2019/07) @paladinzh (2020/05)

@7omb
Copy link
Contributor Author

7omb commented Oct 7, 2022

Changes to an endpoint would only be necessary, if the spec defines response headers. For the servant generator response headers were only for documentation purposes before. I did not check how often other generators are sensitive to response headers. If one uses the e.g. servant generator together with a client generator that is sensitive to response headers, changes would be required.

Edit:
An interesting point is that it is currently not possible to use servants addHeader Funktion since it required the type of the header in the generated API.

@7omb 7omb force-pushed the use-response-headers-in-api-type branch from 20d934c to f32e838 Compare October 19, 2022 15:12
@wing328 wing328 modified the milestones: 6.2.1, 6.3.0 Nov 1, 2022
@wing328
Copy link
Member

wing328 commented Dec 6, 2022

Tests passed locally:

openapi-petstore> configure (lib)
Configuring openapi-petstore-0.1.0.0...
openapi-petstore> build (lib)
Preprocessing library for openapi-petstore-0.1.0.0..
Building library for openapi-petstore-0.1.0.0..
[1 of 2] Compiling OpenAPIPetstore.Types
[2 of 2] Compiling OpenAPIPetstore.API
openapi-petstore> copy/register
Installing library in /Users/williamcheng/Code/openapi-generator/samples/server/petstore/haskell-servant/.stack-work/install/x86_64-osx/d7eaccd255e546d469fd16f5af3892ff7caa92ed8e070d4c3bfe612e97b830c4/9.0.2/lib/x86_64-osx-ghc-9.0.2/openapi-petstore-0.1.0.0-4YkDtr09w237nRoGp23ALC
Registering library for openapi-petstore-0.1.0.0..
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  56.690 s
[INFO] Finished at: 2022-12-06T23:47:57+08:00
[INFO] ------------------------------------------------------------------------

Let's give it a try.

Thanks for the PR.

@wing328 wing328 merged commit f2321a6 into OpenAPITools:master Dec 6, 2022
@7omb 7omb deleted the use-response-headers-in-api-type branch December 6, 2022 20:47
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.

None yet

2 participants