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

Duplicated properties in objects with allof, which makes all properties optional #101

Closed
MingboPeng opened this issue Apr 25, 2020 · 6 comments · Fixed by #103
Closed

Duplicated properties in objects with allof, which makes all properties optional #101

MingboPeng opened this issue Apr 25, 2020 · 6 comments · Fixed by #103
Assignees
Labels
bug Something isn't working

Comments

@MingboPeng
Copy link
Member

MingboPeng commented Apr 25, 2020

In all objects with inheritance, it's "properties" field is duplicated at top level (outside of allof), which should not exist. This also makes all properties optional while generating c# client.

I am not sure if this "additionalProperties" is a property or comes with OpenAPI.

cc: @mostaphaRoudsari

image

@MingboPeng
Copy link
Member Author

@mostaphaRoudsari this might be why our current viewer doesn't like this json file.

@mostaphaRoudsari
Copy link
Member

@MingboPeng, can you help me to find this issue by letting me know which file are you looking at?

additionalProperties is a key in OpenAPI: https://swagger.io/docs/specification/data-models/dictionaries/

@mostaphaRoudsari mostaphaRoudsari self-assigned this Apr 25, 2020
@mostaphaRoudsari mostaphaRoudsari added the bug Something isn't working label Apr 25, 2020
@mostaphaRoudsari
Copy link
Member

I'm seeing it now in the one with inheritance. You might be right. I will give it a try. The documentation is currently broken after the most recent commits. I'm also working on that.

@MingboPeng
Copy link
Member Author

@mostaphaRoudsari Thanks, if I am reading this OpenAPI doc correctly:
additionalProperties should be in this level, right?
image

In terms of how additionalProperties should be defined:
currently, we don't define any type to this, which means it;s type is object, it can be anything. I am not sure if this will work correctly when translating to json. Should we specify it is a string-to-string dictionary?

@mostaphaRoudsari
Copy link
Member

Hi @MingboPeng, See the attached file. This should address both problems that you have raised here:
model_inheritance.zip

Let me know if you see any other issues.

I will push the fix once I can figure out what is going on with the ModelRadianceProperties.

As for additionalProperties we set it to false to not allow users add any other properties than what we have declared.

@MingboPeng
Copy link
Member Author

@mostaphaRoudsari thanks, this works well,

For additionalProperties, I got mixed with userData. I will open another issue for that case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants