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

Added feature to set schema mappings when generation clients. #846

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

denvitaharen
Copy link

@denvitaharen denvitaharen commented Nov 7, 2024

When we tried to migrate from specifying our own openapi generator in Maven to Quarkus OpenAPI generator we realized that we missed the possibility two specify schema mappings: https://openapi-generator.tech/docs/customization/#schema-mapping

Its possible to specify that in the OpenAPI generator, so it was just to expose it so you can set it in the application.properties.

Updated the type mapping test and updated the documentation, hopefully I did all I supposed to do, this is my first attempt to help in an public repo :)

  • You have read the contributors guide
  • Your code is properly formatted according to our code style
  • Pull Request title contains the target branch if not targeting main: [0.9.x] Subject
  • Pull Request contains link to the issue
  • Pull Request contains link to any dependent or related Pull Request
  • Pull Request contains description of the issue
  • Pull Request does not include fixes for issues other than the main ticket

@denvitaharen denvitaharen requested a review from a team as a code owner November 7, 2024 22:22
@denvitaharen
Copy link
Author

Strange that the build failed, it (as usual) worked when I tried it with mvn clean install, I'm looking in to why it doesn't work.

@hbelmiro
Copy link
Contributor

hbelmiro commented Nov 8, 2024

@denvitaharen take a look at https://github.com/quarkiverse/quarkus-openapi-generator/blob/main/CONTRIBUTING.md#resteasy-reactive-and-resteasy-classic for testing RESTEasy Reactive locally.

@denvitaharen
Copy link
Author

Sorry, my bad! I didn't noticed the different profiles, probably to tired to understand :)

I have updated the test, rerun the test suite and it works now.

@denvitaharen
Copy link
Author

I updated the definition of YearMonth in the OpenAPI yaml. It was incorrect, it represented an LocalDateTime instead of an YearMonth. It doesn't change anything in the functionality, it's more about having a correct OpenAPI specification.

Thanks to my colleague @sajox that saw the error.

Comment on lines 147 to 149
If you have an object you want to map to an datatype, you can use the schema mappings. For instance, if your schema is based
on `java.Time.YearMonth`, it will be mapped to an string when generating. If you want to convert the object `YearMonth`then
you can use the following settings:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If you have an object you want to map to an datatype, you can use the schema mappings. For instance, if your schema is based
on `java.Time.YearMonth`, it will be mapped to an string when generating. If you want to convert the object `YearMonth`then
you can use the following settings:
If you have an object you want to map to a datatype, you can use the schema mappings. For instance, if your schema is based on `java.Time.YearMonth`, it will be mapped to a string when generating. If you want to convert the object `YearMonth,` then you can use the following settings:

Can you please rephrase this paragraph? I'm not sure if I understood what you meant here.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the feedback and I understand what you mean, I also had problems understanding want I meant. I tried to rephrase the paragraph but I was never happy about it, but when I read the whole section one more time, I felt that it was enough with an example and description on how to use it (in the table) and then an link to the documentation on OpenAPI page instead.

It felt more right and easier to understand.

If you feel that this is not the way to go then I will happily try to write an new text :)

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.

3 participants