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

Feature openapi_extensions with helper functions for JSON responses and requestBodies #240

Merged
merged 3 commits into from
Aug 4, 2022

Conversation

erewok
Copy link
Contributor

@erewok erewok commented Aug 1, 2022

Following our discussion on adding some helper functions for documenting a JSON requestBody or response, this PR includes the following, all gated by the feature-flag suggested in the discussion, openapi_extensions:

  • New Trait RequestBodyExt for easily documenting JSON requestBody content (implemented for RequestBodyBuilder ).
  • New Trait ResponseExt for easily documenting JSON response content (implemented for ResponseBuilder).
  • Includes tests for new traits and methods
  • Includes documentation for these traits

In addition, the following changes are also included:

  • Adds from_response_name for Ref struct for "#/components/responses/SomeObject" references
  • Adds -F openapi_extensions to cargo tests github actions workflow

Questions

Having traits in this case may be unnecessary because these methods are only implemented in each case for a single struct. We could do one of the following [with these traits]:

  • Create a single trait that is shared by RequestBodyBuilder and ResponseBuilder (instead of two separate traits). The trait could include common shared things, but may result in people using response objects in requests or otherwise blending the two different types. (A single shared trait is originally how I did this in my own project.)
  • Drop the traits and implement these as methods directly on RequestBodyBuilder and ResponseBuilder.
  • Alternatively, I could keep the two new traits but also implement RequestBodyBuilderExt (probably renaming the trait) for RequestBody and ResponseBuilderExt for Response (also renaming the trait) instead of just for the builders and then having traits would probably be justified.

Happy to implement one of the above or leave this as-is. Also, happy to take any feedback or make any changes requested.

@erewok erewok changed the title Feature `openapi Feature openapi_extensions with helper functions for JSON responses and requestBodies Aug 1, 2022
@erewok erewok force-pushed the openapi_ext_v1 branch 4 times, most recently from f321c0f to 0ce10e5 Compare August 1, 2022 04:04
@juhaku
Copy link
Owner

juhaku commented Aug 3, 2022

  • Create a single trait that is shared by RequestBodyBuilder and ResponseBuilder (instead of two separate traits). The trait could include common shared things, but may result in people using response objects in requests or otherwise blending the two different types. (A single shared trait is originally how I did this in my own project.)
  • Drop the traits and implement these as methods directly on RequestBodyBuilder and ResponseBuilder.

I think the two seprate traits are good since then it allows having possible separete set of helpers for each type which is a good thing for separating consern.

  • Alternatively, I could keep the two new traits but also implement RequestBodyBuilderExt (probably renaming the trait) for RequestBody and ResponseBuilderExt for Response (also renaming the trait) instead of just for the builders and then having traits would probably be justified.

Yeah these could be implemented for Response and RequestBody as well thus naming the traits as ResponseExt and RequestBodyExt is reasonable.

Copy link
Owner

@juhaku juhaku left a comment

Choose a reason for hiding this comment

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

This looks awesome 👍 can't wait to get more utilities to the library.

@erewok
Copy link
Contributor Author

erewok commented Aug 4, 2022

Sounds great, @juhaku! Thanks for the suggestions.

I have now made the changes discussed above:

  • Rename traits: ResponseExt and RequestBodyExt
  • Implement RequestBodyExt for RequestBody
  • Implement ResponseExt for Response

I think this PR is ready for one more review and if you are satisfied with it, we can merge. Alternately, I am happy to make any other changes you would prefer.

src/openapi/response.rs Outdated Show resolved Hide resolved
Copy link
Owner

@juhaku juhaku left a comment

Choose a reason for hiding this comment

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

Super. 🥇

@juhaku juhaku merged commit a7907ee into juhaku:master Aug 4, 2022
@erewok erewok deleted the openapi_ext_v1 branch August 4, 2022 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants