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

Request parameter object is shown incorrectly on UI #268

Closed
martin-tarjanyi opened this issue Dec 14, 2019 · 4 comments
Closed

Request parameter object is shown incorrectly on UI #268

martin-tarjanyi opened this issue Dec 14, 2019 · 4 comments
Labels
invalid This doesn't seem right

Comments

@martin-tarjanyi
Copy link

Request parameter object of GET request is shown as JSON on swagger-ui instead of individual parameters.

image

This issue has been mentioned a few times before:

These all have been closed as they are caused by an issue in swagger-ui project. However, that issue was opened 18 months ago and there is not much activity on that project so I don't expect it to be fixed anytime soon (if ever).

Would it be possible to generate parameters in the OpenAPI docs such a way that it can be correctly presented by swagger-ui?

I created a demo project with 2 endpoints:
https://github.com/martin-tarjanyi/openapi-ui-issue/blob/master/src/main/java/com/example/openapiuiissue/DemoController.java#L13-L23

As I understand semantically there is no difference between them. However, the generated API docs (and in consequence the UI) is quite different.

I'm rising this issue again because this is the only thing which prevents me from moving all my projects to springdoc. I also would be happy to contribute to solve this issue if possible.

@springdoc
Copy link
Collaborator

Hi @martin-tarjanyi,

You can achieve the same result using the swagger-annotations:

@Parameters({@Parameter(in = ParameterIn.QUERY, name = "name", schema = @Schema(type = "string"), required = true),
		@Parameter(in = ParameterIn.QUERY, name = "age", schema = @Schema(type = "integer", format = "int32"), required = true)})
@GetMapping("1")
public String demo(@Parameter(hidden = true) MyParameterObject parameterObject) {
	return "demo";
}

If you have a better proposal, feel free to contribute.

Please don't open many issues for the same topic. You can add your comment, even the issue is closed and we will reopen it, if needed.

@daniel-frak
Copy link

daniel-frak commented Jan 13, 2020

Doesn't the proposed solution violate DRY and KISS, though? If MyParameterObject were to be reused in different endpoints, the @Parameters would have to be repeated. The addition of examples, descriptions etc. would further complicate the annotation and make the code more unreadable. Furthermore, AFAIK @Parameters won't annotate MyParameterObject in the "Components/Schemas" part of the specification, once again forcing the dev to duplicate code/documentation.

Granted, one could probably make a new annotation like @MyParameterObjectSchema but forcing the developer to remember potentially tens of different annotations for all the DTOs doesn't seem like a good idea :(

While I would much rather the underlying issue be fixed in Swagger UI, I don't think implementing a sort of "unwrap object parameters" option to Springdoc OpenAPI is a bad idea.

What do you think?

@bnasslahsen
Copy link
Contributor

Hi @daniel-frak,

What you are proposing is a good pattern in case of repeatable objects.
Its already for some commons spring Objects.
You can have a look for example to: @PageableAsQueryParam object in springdoc-openapi-data-rest.

For objects that are related to your application, this design can also be applied, but you should refactor the relevant annotations on your application side.
And if you face, any reusable spring object, please feel free to contribute.

@daniel-frak
Copy link

@bnasslahsen, I'm afraid you completely misunderstood me. Please, take a second look at my comment. This is not about proposing new annotations.

What I'm proposing is that this is actually a terrible pattern for the use case described in this issue. In many cases, the proposed solution forces the developer to annotate every single GET endpoint (presuming every endpoint would have an object as a parameter, which can happen).

For objects that are related to my application, I should be able to just annotate the objects themselves, instead of having to rely on terrible, unwieldy and hard to maintain workarounds. The @PageableAsQueryParam is fine if you have one or two "special" objects. It's not ok when we're talking about every single object in the GET API.

Nevertheless, I feel like we are getting off-topic discussing this. I feel like we should be discussing whether or not the actual proposed solution (that is, unwrapping object parameters in GET endpoints, toggle'able via config option) is a good enough idea and how simple it would be to implement.

My opinion is that this is a good idea, making code much easier to maintain than the annotation-based alternative and well worth looking into. The biggest benefit is that after the Swagger UI fix, if it ever comes, all that will be needed to get objects working normally will be a single toggle, instead of a maybe-complete rewrite of documentation code (as with the annotation-based solution). Unfortunately, I can't speak for how hard it would be to implement.

As an aside, a toggle like this would also be a valid workaround for #4404.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

3 participants