feathers-swagger
feathers-swagger copied to clipboard
Is support for pagination and 'multi' create needed?
I'd love to create a PR to support pagination: https://docs.feathersjs.com/api/databases/common.html#pagination
As well as to add support for 'multi' create: https://docs.feathersjs.com/api/services.html#create-data-params
If I'm not missing anything, these need to be added. Can I go ahead, and if so, will I get the required attention to merge the code?
Hi,
I also thought about adding support for pagination out of the box for the next version. But it would be a breaking change if not hidden behind an option.
You can already achieve it by customizing the defaults.operationsGenerator for find. So the first step would be to create that, this could be a documentation or FAQ entry for the current version and then used for the next version.
For the create multi I intentionally added no support, because in comparison to the other multi operations it is exactly the same endpoint as create. So you can only change the requestBody and response of the current create operation to show arrays. At the time I implemented it, there was no real swagger support for supporting multiple types. And at least swagger-ui does not support it well enough I think to have both single and multi creation documented at the same time. Theoretically, it would be the usage of https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/, also this can be changed by using a defaults.operationsGenerator for create.
I am open for any thoughts.
Wow, thanks for the great comment. I agree that this could be accomplished mostly with doc/examples, and that's a better option for this verion. I think both would be helpful, especially since pagination is supported out of the box with feathers. If you think it's valuable, instead of implementing anything in the lib, I will simply add some doc and examples around pagination (and multi-create as well if I can find a decent solution). Does that sound OK to you?
EDIT:
Looking into it more, I'm not so satisfied with the operationsGenerator implementation. The problem I personally see is, that function doesn't have a model name as input, which TbH seems like the best way to implement this.
Wouldn't it be fairly simple and non-breaking to add a boolean "paginated" property to ServiceSwaggerOptions (similar to the existing multi) and in the default implementation of the schema list generation, make the list paginated instead:
https://github.com/feathersjs-ecosystem/feathers-swagger/blob/81dd8a3e17438ddbb4276d8199e07137fcc561b4/lib/v3/generator.js#L4
Then users could simply add paginated: true
and get this nice behaviour.
The line you mentioned is not the one for the operationsGenerator.
https://github.com/feathersjs-ecosystem/feathers-swagger/blob/179fa50856c785e9984ae02847bae3d083fc3ca0/types/index.d.ts#L56-L68
are the available options you can consume in a custom operations generator.
The relevant code:
https://github.com/feathersjs-ecosystem/feathers-swagger/blob/179fa50856c785e9984ae02847bae3d083fc3ca0/lib/openapi.js#L148
https://github.com/feathersjs-ecosystem/feathers-swagger/blob/179fa50856c785e9984ae02847bae3d083fc3ca0/lib/openapi.js#L65-L91
https://github.com/feathersjs-ecosystem/feathers-swagger/blob/179fa50856c785e9984ae02847bae3d083fc3ca0/lib/openapi.js#L155
https://github.com/feathersjs-ecosystem/feathers-swagger/blob/179fa50856c785e9984ae02847bae3d083fc3ca0/lib/openapi.js#L175-L184
and
https://github.com/feathersjs-ecosystem/feathers-swagger/blob/179fa50856c785e9984ae02847bae3d083fc3ca0/lib/openapi.js#L205
So there is really all information you can rely on available I would say.
For pagination, you are right that such an option could be added, but my plan was to use the service.options directly, so that no extra option would be needed. This can also be read in a operationsGenerator.
Edit: And of course it would be fine to add examples and documentation. Also PRs for the planned changes would be welcome.
The line you mentioned is not the one for the operationsGenerator.
I understand that. What I was trying to say is we could easily add a default pagination implementation in that code... but I like your idea to do it in the operationsGenerator better. I had missed that we had the required params there.
For pagination, you are right that such an option could be added, but my plan was to use the service.options directly, so that no extra option would be needed. This can also be read in a operationsGenerator.
Oh, you're absolutely right. So I guess this is what you meant by breaking change... because if we read the actual options here, suddenly pagination would start happening for people in the swagger without them taking any action other than updating. I see how that could be an issue.
I will be implementing basically what you describe for pagination in my own service. Once that's done, I will make a PR here since I think the code will be somewhat similar (changing the default service generator). Are you saying you'd be interested in that even though it might constitute a breaking change?
Yeah, I would be interested.
Sadly it could take some time to land in a version 2.0
Support has been added.