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

Add single value enum when using const keyword for compatiblity #413

Conversation

uasouz
Copy link
Contributor

@uasouz uasouz commented Aug 22, 2024

Use extra single value enum where const is used due to lack of support from jsonschema2pojo

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it:
jsonschema2pojo does not support const keyword yet.
This was causing a bug when using resusable tasks that have no value on with block. It would always match to CallAsyncAPI.

In order to achieve proper behaviour I added a single value enum below each const. This will make it compatible with jsonschema2pojo while they solve the issue #1001.

joelittlejohn/jsonschema2pojo#1001.

Also addded a test to validate the behaviour

Special notes for reviewers:
I think this may be a good idead to adopt as a single enum on the main spec too. const and single value enum are the same thing under the hood, for jsonschema2pojo at least.

Additional information (if needed):

@uasouz uasouz force-pushed the fix/jsonschema2pojo-lacks-const-support branch 2 times, most recently from b97121c to 898be3a Compare August 22, 2024 22:54
…upport from jsonschema2pojo

Signed-off-by: Vinícius Moraes Lopes <[email protected]>
@uasouz uasouz force-pushed the fix/jsonschema2pojo-lacks-const-support branch from 898be3a to d316319 Compare August 23, 2024 00:22
@fjtirado
Copy link
Collaborator

@uasouz I think we should add support to const.
To implement this workaround, we should change the original schema in the spec repo (the main target is for them to not differ in a single comma, actually copying the schema was a workaroung till pojo2scheme add support for non local file paths)

@uasouz
Copy link
Contributor Author

uasouz commented Aug 26, 2024

@fjtirado 100% agree, we should.
I will then open another PR on the main specification repository for the workaround.
So if it is ok, you can close this PR.

@fjtirado
Copy link
Collaborator

fjtirado commented Aug 26, 2024

@uasouz Actually I believe we should use another PR to add support to const. The idea will be to incorporate to our serializer the const check. so, if string is different from http, HttpTask deserializer will fail. I can work on that, so I will open the issue and the PR, you do not need to change the origina schema.

@fjtirado
Copy link
Collaborator

fjtirado commented Aug 26, 2024

@ricardozanini Please close this issue and assign #418 to me

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