Skip to content

Missing extension to schema property #622

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

Closed
raghuraman1 opened this issue Apr 27, 2020 · 14 comments
Closed

Missing extension to schema property #622

raghuraman1 opened this issue Apr 27, 2020 · 14 comments
Labels
bug Something isn't working

Comments

@raghuraman1
Copy link

raghuraman1 commented Apr 27, 2020

Hi @bnasslahsen ,

Please see https://github.com/teq-niq/sample/tree/extending3
In the person schema for worth field am expecting a extension x-Currency with value USD.
I am trying to show using swagger extensions validation constraints that swagger or springdoc might not be showing. This includes even custom constraints.

In springdoc-openapi-webmvc-core\src\main\java\org\springdoc\webmvc\api\OpenApiResource.java I debugged this method - openapiJson()

I can see the extension with correct value in this object
OpenAPI openAPI = this.getOpenApi();

But the extension is missing in the api-docs json. The other extensions are all appearing properly.
problem

Attaching the json txt:
json.txt

Raghu

@bnasslahsen
Copy link
Collaborator

@raghuraman1,

This is due to the fact that MonetaryAmount is a complex object wich is by default resolved using $ref.
When an object has $ref, No sibling values are allowed.

If an application wants to enable the springdoc-openapi support, it declares:

SpringDocUtils.getConfig().replaceWithClass(MonetaryAmount.class, org.springdoc.core.converters.MonetaryAmount.class);

In order to enable the extension for a complex object, it shouldn't be resolved as $ref; If you declare the following, it should work.

SpringDocUtils.getConfig().replaceWithSchema(MonetaryAmount.class, new ObjectSchema()
				.addProperties("amount", new NumberSchema()).example(99.96)
				.addProperties("currency", new StringSchema().example("USD")));

PS: Please don't open many issues on the same topic. In this case are related to add the MonetaryAmount on springdoc. #621, #622, #617, #606.
Even an issue is closed, if someone adds a comment, we keep following it.

@raghuraman1
Copy link
Author

raghuraman1 commented Apr 28, 2020

Hi @bnasslahsen
Was wondering where you disappeared and hoped you are fine. :) Gud to hear from you again.
Noted about " don't open many issues on the same topic."
My motive was to avoid confusion.But what you said makes sense from the perspective you mentioned. Will abide by that.
I will try what you suggested and get back tomorrow.
I suspected it had something to do with ref.

One doubt:
Any complex property will tend to be a ref- Right (because you are referring to the schemas in components)? If so and if i need ability to add extensions to any complex property this looks complicated. If that's correct can it be simplified?
Also the extensions are being added to the property and not to the type. So seems like if not siblings at least extensions should be possible. Point is the extension is clearly visible in the object OpenAPI model instance. Just not there in the json. Wish it would persist in the json.

R

@bnasslahsen
Copy link
Collaborator

@raghuraman1,

The rendering of Complex properties is not done by springdoc-openapi, but using swagger-core.
You can test it with the following:

ResolvedSchema resolvedSchema = ModelConverters.getInstance()
		.resolveAsResolvedSchema(new AnnotatedType(Person.class));
if (resolvedSchema.schema != null) {
	Schema schemaN = resolvedSchema.schema;
	Map<String, Schema> schemaMap = resolvedSchema.referencedSchemas;
}

I don't see how you can get extension on complex properties with the current swagger-core implementation.
But, if you find a way, please feel free to share.

@raghuraman1
Copy link
Author

@bnasslahsen
Have 2 solutions. Will get back

@bnasslahsen
Copy link
Collaborator

@bnasslahsen,

I was going to release v1.3.5. If its a minor change, i can wait.
Do your solutions require any change on springdoc-openapi?

@raghuraman1
Copy link
Author

raghuraman1 commented Apr 28, 2020

@bnasslahsen
You carry on with 1.3.5. This can wait. Lots of new features people are waiting for.Lets not prolong the wait.
But I will outline what I see. There are two ways:
A) In OpenApiResource.openapiJson() we can provide a different ObjectMapper than what we get from io.swagger.v3.core.util.Json.mapper(). All the code is there in swagger-core we need a small change after copying their code. We need not copy. Just provide our own Json.mapper method.
B) In io.swagger.v3.core.jackson.SchemaSerializer in swagger-core we change this
image
I vote for B. And we try convincing swagger guys like last time. I can look at what code they have in their latest and try raising a PR.
What do you say? We do have a requirement.

@raghuraman1
Copy link
Author

@bnasslahsen
I can even attempt A let me see how simple or complex its. Will get back

@bnasslahsen
Copy link
Collaborator

@bnasslahsen,

The springdoc-openapi relies on swagger-core. Option B, is the better choice.

@raghuraman1
Copy link
Author

@raghuraman1
I will write an implementation and raise a PR on swagger-core. Do give your support like the last time.
Thanks.

@raghuraman1
Copy link
Author

raghuraman1 commented Apr 28, 2020

@bnasslahsen ,
The code changes I tried on io.swagger.v3.core.jackson.SchemaSerializer.
They cause proper json and yaml.
There is a little rabbit hole. Looks like ui- the reactjs code will also need improving.
I can attempt that also after this.
Meanwhile for your opinion attaching the following
json:
api-docs.json.txt
yaml:
api-docs.yaml.txt
io.swagger.v3.core.jackson.SchemaSerializer code changes:
image

@raghuraman1
Copy link
Author

raghuraman1 commented Apr 29, 2020

@bnasslahsen
I have further simplified io.swagger.v3.core.jackson.SchemaSerializer code changes as shown above.
I hope you give a thumbs up on swagger-api/swagger-core#3539
I have also detailed different samples and different outputs in the PR
I am hoping it gets merged.

@breno-ki
Copy link

@raghuraman1,

This is due to the fact that MonetaryAmount is a complex object wich is by default resolved using $ref.
When an object has $ref, No sibling values are allowed.

If an application wants to enable the springdoc-openapi support, it declares:

SpringDocUtils.getConfig().replaceWithClass(MonetaryAmount.class, org.springdoc.core.converters.MonetaryAmount.class);

In order to enable the extension for a complex object, it shouldn't be resolved as $ref; If you declare the following, it should work.

SpringDocUtils.getConfig().replaceWithSchema(MonetaryAmount.class, new ObjectSchema()
				.addProperties("amount", new NumberSchema()).example(99.96)
				.addProperties("currency", new StringSchema().example("USD")));

PS: Please don't open many issues on the same topic. In this case are related to add the MonetaryAmount on springdoc. #621, #622, #617, #606.
Even an issue is closed, if someone adds a comment, we keep following it.

FYI the first solution is outdated (also in the FAQ https://springdoc.org/faq.html#how-can-i-use-enable-springdoc-openapi-monetaryamount-support-), the class that should be used is org.springdoc.core.converters.models.MonetaryAmount.class.

Using sprindoc 1.5.6 here, still get the error without manually setting the converter. I believe this was mentioned to be an issue with swagger-core, is there any update on this?

@bnasslahsen
Copy link
Collaborator

@breno-ki,

Doc is now updated. Note that you can propose PR for it which is welcome in Open Source projects.
You can see this sample test for how this is applied.

@breno-ki
Copy link

breno-ki commented Apr 1, 2021

@breno-ki,

Doc is now updated. Note that you can propose PR for it which is welcome in Open Source projects.
You can see this sample test for how this is applied.

Lovely, cheers for fixing! :)

@bnasslahsen bnasslahsen added the bug Something isn't working label Jan 10, 2022
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

No branches or pull requests

3 participants