Avoid unconditional COUNT(*) query on every SCIM search/list request - #51570
Avoid unconditional COUNT(*) query on every SCIM search/list request#51570elhichaouiYassine wants to merge 1 commit into
Conversation
Closes keycloak#51402 Signed-off-by: elhichaouiYassine <y.elhichaoui@gmail.com>
There was a problem hiding this comment.
Pull request overview
Optimizes SCIM pagination by avoiding unnecessary count queries for partial result pages.
Changes:
- Normalizes pagination parameters centrally.
- Adds provider capability signaling for count optimization.
- Updates user and group providers to consume normalized pagination.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
scim/services/.../ScimResourceTypeResource.java |
Computes totals from partial pages when possible. |
scim/model/.../UserResourceTypeProvider.java |
Uses normalized user pagination. |
scim/model/.../GroupResourceTypeProvider.java |
Uses normalized group pagination. |
scim/core/.../ScimResourceTypeProvider.java |
Adds count-optimization capability API. |
scim/core/.../AbstractScimResourceTypeProvider.java |
Enables optimization for model-backed providers. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if(supportsCountOptimization && resourceSize < searchRequest.getCount()) { | ||
| totalResults = (long) (searchRequest.getStartIndex() - 1 + resourceSize); |
There was a problem hiding this comment.
Valid finding from Copilot and with a good suggestion. This handles both when you page past the end and when you get 0 results starting from startIndex == 1 where we can decide for sure that the count is 0.
sguilhen
left a comment
There was a problem hiding this comment.
Thanks for the PR @elhichaouiYassine . Copilot has flagged a valid finding, and I would suggest to both fix it and include a test case for the out-of-range pagination case.
One minor observation: normalizePagination() is only called when supportsCountOptimization` is true, which means pagination normalization and count optimization are implicitly coupled. Since they're independent concerns, it might be cleaner to always normalize.
| if(supportsCountOptimization && resourceSize < searchRequest.getCount()) { | ||
| totalResults = (long) (searchRequest.getStartIndex() - 1 + resourceSize); |
There was a problem hiding this comment.
Valid finding from Copilot and with a good suggestion. This handles both when you page past the end and when you get 0 results starting from startIndex == 1 where we can decide for sure that the count is 0.
Closes #51402