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

REST Data with Panache paging parameters should be visible at Swagger/OpenAPI #11391

Closed
alievrenkut opened this issue Aug 14, 2020 · 13 comments · Fixed by #11969
Closed

REST Data with Panache paging parameters should be visible at Swagger/OpenAPI #11391

alievrenkut opened this issue Aug 14, 2020 · 13 comments · Fixed by #11969

Comments

@alievrenkut
Copy link

Description
I created a PanacheEntityResource and generated OpenAPI description but pagination parameters are not visible to Swagger so I can not give pagination parameter from Swagger UI. I also generated Typescript API from it using swagger-typescript-api. I can not see paging parameters from Typescript side too.

Could you add necessary info at RestDataResource?

@alievrenkut alievrenkut added the kind/enhancement New feature or request label Aug 14, 2020
@quarkusbot
Copy link

@phillip-kruger
Copy link
Member

Hi @alievrenkut . Thanks for this issue. Do you perhaps have a small project I can look at ?

@alievrenkut
Copy link
Author

Hi you can clone: https://github.com/alievrenkut/quarkus-rest-data-panache which is sample project I created with quarkus.

You can see that after I run "gradlew quarkusDev" you will see that there is no page parameters:

image

@phillip-kruger
Copy link
Member

phillip-kruger commented Aug 14, 2020

Hi @alievrenkut - Thanks for this. This is a feature request to the quarkus-rest-data-panache extension.
The normal JAX-RS code generated by this extension does not contain any OpenAPI annotations. And because the pagination used query parameters, it's not automatically generated with OpenAPI. (It does not generate code with @QueryParam, but rather get the info of the UriInfo)

I would say the panache maintainers needs to look at how to do this best. I would suggest :

  1. Check if OpenAPI extension is included
  2. If so, and paging is active on a list method, add something like this in the generated code:
@Parameter(name = "page", description = "The page number", in = ParameterIn.QUERY, schema = @Schema(implementation = Integer.class))
@Parameter(name = "size", description = "The number of items per page", in = ParameterIn.QUERY, schema = @Schema(implementation = Integer.class)

@FroMage - w.d.y.t ? I am happy to do the PR if you agree with the approach. Let me know.

@FroMage
Copy link
Member

FroMage commented Aug 17, 2020

I would have to remember why @gytis didn't use @QueryParam first.

@gytis
Copy link

gytis commented Aug 17, 2020

Because the pagination can be disabled, I have decided to use UriInfo instead of having unused method parameters in an interface. However, if it is causing more issues that solving, maybe it is worth reconsidering the approach?

@phillip-kruger
Copy link
Member

I think we can still use UriInfo, just also add the OpenAPI annotations when pagination is possible. (And OpenAPI extension is added)

@FroMage
Copy link
Member

FroMage commented Aug 17, 2020

Isn't it simpler to add query parameters (since they're always optional) rather than include an optional dependency on OpenAPI?

@phillip-kruger
Copy link
Member

Yes definitely !

@gytis
Copy link

gytis commented Aug 17, 2020

I'm ok with adding the query params in place of the UriInfo. The only issue is that the method definition will get longer and the user will have to copy it all if he'll want to customise it. For example:

public interface MyEntityController extends PanacheEntityResource<MyEntity, Long> {
    @MethodProperties(path = "/some-custom-path")
    Response list(int page, int pageSize, String sort);
}

@FroMage
Copy link
Member

FroMage commented Aug 17, 2020

I think that's fine, most IDEs can do that if you add the @Override and it will be checked too.

@gytis
Copy link

gytis commented Aug 17, 2020

OK, assign this issue to me then and I'll sort it out.

@FroMage
Copy link
Member

FroMage commented Aug 17, 2020

Your wish is my command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants