Initial pagination in the admin REST API for identity providers #22003
Conversation
ghost
added
| @Operation( summary = "Get identity providers") | ||
| public Response getIdentityProviders(@Parameter(description = "Provider id") @PathParam("provider_id") String providerId) { | ||
| @Operation( summary = "Get the identity provider factory for that provider id") | ||
| public IdentityProviderFactory getIdentityProviders(@Parameter(description = "The provider id to get the factory") @PathParam("provider_id") String providerId) { |
There was a problem hiding this comment.
Minor: Thanks for updating the outdated javadoc and description. But method name is still slightly outdated. This minor change to update it.
| public IdentityProviderFactory getIdentityProviders(@Parameter(description = "The provider id to get the factory") @PathParam("provider_id") String providerId) { | |
| public IdentityProviderFactory getIdentityProviderFactory(@Parameter(description = "The provider id to get the factory") @PathParam("provider_id") String providerId) { |
| @Produces(MediaType.APPLICATION_JSON) | ||
| @Tag(name = KeycloakOpenAPI.Admin.Tags.IDENTITY_PROVIDERS) | ||
| @Operation( summary = "Count identity providers") | ||
| public Integer countIdentityProviders( |
There was a problem hiding this comment.
Sorry I did not spot it before, but do we really need count endpoint?
I know that we have "count" for some objects, but for instance, I've checked clients (which already support pagination). And the ClientsResource does not have any "count" endpoint for clients. It seems that admin console doesn't need it as it does not display the count of search results and count of pages for clients. It just display arrow for next page (if exists).
IMO if admin console doesn't need "count" endpoint, then my vote would be to skip "count" endpoint. I would just go for necessary minimum to have admin console working (Especially since we may migrate to "Admin REST API v2" and hence adding more unnecessary mess to admin REST API doesn't help...)
There was a problem hiding this comment.
Ah! Okis, the I will remove the count endpoint. I just added it because I cloned the behavior in the users endpoints.
There was a problem hiding this comment.
Yeah, we're not using this at the moment. Perhaps we should, but pagination is just kinda borked in general atm. That won't change until we start working on a proper V2 API.
There was a problem hiding this comment.
Removed the count endpoint!
| @GET | ||
| @Path("instances/count") | ||
| @Produces(MediaType.APPLICATION_JSON) | ||
| Integer count(@QueryParam("search") String search); |
There was a problem hiding this comment.
See my other comment for "count" endpoint
There was a problem hiding this comment.
Removed the count endpoint!
|
@mposolda No problem, we're always happy to review work from the community. |
|
Do we have an issue logged somewhere to implement this in the UI as well? |
Closes #21073
Initial pagination to start working in identity providers. As you can see it's (for the moment) just a wrapper that iterates on the current complete list of identity providers. Once merged the UI team can start working on using the new endpoint and we can also start working on modifying the model API. When UI team finishes moving to the endpoint we can also start not returning the list of providers in realm endpoints. Changes from #21487:
@mposolda @jonkoops @cgeorgilakis FYI