AAP-86117 Prefetch object_role_assignments to eliminate N+1 serializer queries - #1096
Merged
john-westcott-iv merged 1 commit intoAug 11, 2026
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe access-list serializer bulk-fetches and classifies assignments for paginated actors. The view conditionally loads related resources. A regression test checks query growth as the actor count increases. ChangesRBAC access-list query optimization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant UserAccessViewSet
participant Database
participant AccessListSerializer
participant get_object_role_assignments
UserAccessViewSet->>Database: Fetch filtered actors with related resources
UserAccessViewSet->>AccessListSerializer: Serialize paginated actors
AccessListSerializer->>get_object_role_assignments: Request actor assignments
get_object_role_assignments->>Database: Bulk-fetch assignments for parent actor IDs
Database-->>get_object_role_assignments: Return grouped assignments
get_object_role_assignments-->>AccessListSerializer: Return cached actor assignments
``
</details>
<!-- walkthrough_end -->
<!-- pre_merge_checks_walkthrough_start -->
<details>
<summary>🚥 Pre-merge checks | ✅ 4 | ❌ 1</summary>
### ❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
| :----------------: | :--------- | :------------------------------------------------------------------------------------ | :--------------------------------------------------------------------------------- |
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
<details>
<summary>✅ Passed checks (4 passed)</summary>
| Check name | Status | Explanation |
| :------------------------: | :------- | :--------------------------------------------------------------------------------------------------------------------- |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly identifies the main change: prefetching object role assignments to eliminate N+1 serializer queries. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
</details>
</details>
<!-- pre_merge_checks_walkthrough_end -->
<!-- finishing_touch_checkbox_start -->
<details>
<summary>✨ Finishing Touches</summary>
<details>
<summary>🧪 Generate unit tests (beta)</summary>
- [ ] <!-- {"checkboxId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Create PR with unit tests
</details>
</details>
<!-- finishing_touch_checkbox_end -->
<!-- tips_start -->
---
Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=ansible/django-ansible-base&utm_content=1096)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
<details>
<summary>❤️ Share</summary>
- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai)
- [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai)
- [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai)
- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
</details>
<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>
<!-- tips_end -->
<!-- internal state start -->
<!-- N4IgzgxgFgpgtgQwGowE5gJYHsB2IBcAjADTgAuqArhGZajACYDKZCZMBoYF1t9K6bHiKkADqgDyAIwBWMGhgBuMMARABidQAIACgCUtYSnESoAngB0cV/VvWEADAE4AbMS2AcAgCCXnQFoADhdCQgB2XXoAMxgyaC0sWXkyAH1ULAAbGGSEMEwAcxw4GBwyMC0yLC0YdIw4DBw2GC0AOQBqQkM0DAQagC80LQBHSi6VYkBcAi1xGGjY2DKe9K00zK0c/MLi0q1IrFRy2HKEVDyY+MSaLXqtAAMAVTA0LwgIFTAkDBgAdyYYgDpTilHqhun00MkILh2AAPMgACgAlDc1jgGMsYJRHmUyLA4FoGHR6nlOiCehhemwhH8tAAVKAYBaKLAYBhlURoPyYgYMNgIKQ5JrDUZla43Z6vXIAGQZZAAshhofUATFkgk5DRUhksusMAUiiUwIibn8rFY6U0sKIyLVyZTcFopMUZhhtlIsDjbg8ni83h9vr8yMiEKjbjSYAg4OLfZ8fjFkTi0pQ8lADjAMPswFBjowptmSloIOl1tSAJJkK44QuUBgqIYjcx+SGUfOQ5SoBCnFFo+iseoLHaLfkQADW5Uq7NQnOBdeFWi+sBwUyiMWgOZ1eq2C3oWmbCEUCAwRakmRN1hwOjSkUPRPRoj27DRwNBtut9utRXx8noORvkTSeIAFj+ABWAAmAJOkhVFsUqBw/gcZxINwVl3C+F0UyfMkKVfRchRBWsawgb9MBwYk/ywPFQMIABmMctDg00cE0LQAGEKP1bYjBMY5LDPJgIB6GBJT5AsKNEY4GVwMp6DvVAH0MSgpG4YNrR6K44HELBlA4spdn2LlJwQH1ciqVE73qUpqQAMT2W4ViyAzsmMsAAHpM2zBg/j2PJgxfIRkTQj0EFETTFUQdh0jMLRQmIBCHB3YEwHcegwDvHBHnKWpa1qTTlDRci8QCEDgMAZAInD+KikOgui4NAhwStqpwqpQ28Uq2G9qNAkqAOo6FDHZRhKFEMBqRuezkkcoyJVchAvmhP4ZASdgNKLdhkRytI8p2f96L+FxQOamDdtAprHiglr50PJoxq1ZJ2AjJzppc2b5sWqRltEVaYACnI1hCtIwsaSKd0rLNSMYU8rAAdT2Yd0iwBA0U0x0ykzLAvhzDatPgTc1iIrATMWcoVHkm7Mgm4FHreZFigYcyDXceoqwYG8bnc+hPO83zsP81D6TiQjiJzAq/tC2ogai0I/lCaiyr+JxwjO5CjrgsJSrggCFcO6lZQJ8syaye64Cp3IgxDegvrRGpuFMunmQNOc0CaYKxfC6ooubaBg1OTytC8HZvlt+ntgx7dTGHHMvj2R53CkShyxxJoZPvHMWciaJ6ErWtQ6aZsa32Or4rgBZzfgA8cEj9DrimmArT5VYvsrKLgzRUw8nqPwsEiPw0DSfZ23BkaQFIdsfmMUwzFlYLOBAYNMGPLJ+UeFzUCHZ7RAwNyuiwtARtEMw1GYrwSz8U4K/beSuIn+JIgLMHTjARivAYGs0RuZI4/SYdklmaBslyXUmwDTxkqJ/Ycfhf4phbgWIsADIhRTVEkFyeR4b8iWPZNYACNwOz0msdISwjIVHQBWNYuZ24NEvtvMEqBRqAlVBcFI411xANKMiHA6MtBFl6IeKKAlVzYnmEnFQlB0jbDfIccSmdyzW3LJhahXZ0R8HSqmAsdApF4yIYATAIFhYJYWAAA3ComAipuA3gnH4Qhtk8JRXEh6egiA+yYNIZEQcRlhyQyYtoLwoi0B2mURUFRhEiwXyEGULuVRoSyXkrZUQikagQFMtaa0KhGLNFwDARi0oK5lC9uDBg+AtCtBcNRFyfgHCMQAKKmLdmiSENZ0SKBjFUdO94CkAAldRQGHrPdKGAF4fwFCvNewVN6NO+HvA++ANBeJPmfXxl9x48Rvnfb2KSzz3GBFGXIfpYxkGVCkaxjxAxaHYV8UWNRaxs2qEkVI1RGgMDhAAchSlgOgrxHlIjnAuFRlj9hwCwDWJYxi7xYlIWNFQrzUCvGRBbPxDoZh7CEUo38h52Acw0dHak5olzYH2DlTIHE4U9joBXNEicdioqdmiQ5ZxApQFeQnek0k7k4SXDMFcUAPFHx8SEqSdFyVBOOH4sJt9gUpzRDEuJGAEntWSY/M8aSK6ZPqLWXJPsCmtAAqUwglTqn3NEvU+gYyzkzD0mQApkp0bdPYNwbIIUXI2tKEMoy69N6OpNmAZIMiJmHxmafJ0F8cxXyWeEtVayrDP1floAAIjIb2lRrGNmCnwJoOV7wlzRAgdEeQUqYDfCTLQbYMDwJvDiNgKiDJ42mn4GRM5zCiWbOWex5cygV2OMDKCyl8zzmKOiIwoiyg5o4SLA6AS6oJV3lyrxPLhX8sOIK3lyjwlirkjmSVx5pWJJdJ8eVVhFUZLPFk1V99GAauok4Up5SzxVPfPqupQjjXNLNQU2UjAMDGBAAAX1IDWXsmQGBsRrAAIXhiOJgizzB6FWaofAABtUAA9TglgYGoRDWQIDUWAi4AIoFojUQCE4AI3TbFdKme6l2DqSauVXi6kZlHbU11yF6mUPrSDKTkkeggoFSC004/gaibGIOTJAJG1OvIl5NCghXBQ9poGJoElaOgudrQ1DlTsWyRQciElIn7HQJY628NeQaP43T70gawCONiK1jEumE/DL4X7iAIdWch1DqzkigQwxATWwEBIuAgC4YjbBSMgHI/ax11Hhkb3oykRjnrvV/H3t09jZA+OECcE4HjqI+OgUw4J7i5g1CiczQZxsRmm0wBzW8IQxMbZlvLJCDS2YyiVrizWmU6IhTcF0rZA6bYjBlHHQZDNnXKDplrNEUOBxgwUuUGsF+27cBqTwjukzpAzOgeHFZz6NmyDCaKCzD937nPg1c1MtDyRqKRCkMBBALgpDXf8w4ILOI1Bzz6eTCTzqICupcsa1j5BjipZVQQHqDgssMD4z1AT4AhNqGxVeHla4aBWPrGYI5JyOHnUWw0fBHtQVXMyBqWFD4nkvLeTAD5yJcH/MBWUOlYLydQu+msMgFA+nxxgIY6YjTXlgGBi6Ocv1iWoFJXOdCDL0SWz8Ez14nCEYs1ImtkAG2LNbbEpkRUe21D2ccydpDKHzvudCKBO7EAELAUcABFDYhgtvd6f0r7NGft0bkeSSdSW2OsA4yD/jIQId8cw5l2HBXhPFYdCI0czCOKKPwvuJYuDK22SNloEs0akoUp5TeRBlxo+4ykFFGA8fKD6vsiNicdQAF8ugSghIak88OwL+c6V9ck5aimGgSveb0rUj0MIgdaxtyFh1FeNcg7UE9HcEbdwLN6A0HcLZeos+kgKKHUNHMTffni49MGkE/Ruzt5rFeHAOOh7rYBTAczlmNe7eE/SZMeuQBobO0/9zMAYDUSkAEACAQHAwFw6EC9iFu9o7oMs7r9m7v0OgIlgfF7kDoHsEAHr7tRKEABPlhPGoDcHQjnowrdA3qwpjmcvUNujvCNnwvMBHl/H2iItsCLOSpIlsJwjKNohiugMlDECSjkkZLAJmrojpOpvsOShAGoowb8u4NAtMMCMoAIk0DZqYjpkvkoCyCXukH4NYgOPgkOKOD2ouGAqJCUMYuWAyCDHuAeEeCeKZhflfurtZlrvtm+kdgALojxF4xiMBWZ1BkDIYwawYgAm49QwABAMDeauCuAODf4uIf7BGhCRABAQChBODRCRAuBOAODAQMBOAQCRCEDdLARODUQuDyAm4uAAQID/4wAuCRAoHUTnoICgQMChDAShABDUQW4wB5G+YMD+bdIvwIAOCRAODNEZHRAOA1gOCaxSAKyhAuD9FBHgQBDAQEbNHm4ATxGmYASea3YhABbv5OBSAAR/6gShCTHUQwCTFSDTF9GgQPb7R3bgROAgDOEgDTCvonAwAsSwAjgwagAMgSBtgggvzFAEAuLpAxwgAMj6DWHuHsRbA6B6yMDAk9BglowiIMDWH6CImgkwA/osh6DNjRoWYsAgikRgAfHyDDgEA8DYkgAswMB4k4A0guiZBkkjiUlUDUn3pQZSBSAuhQxpDsAXiMlkCZCYlgm0n0nRoqBEQYBWhCAskUn4Aglgk1A4ARwMAli5AjBgBElqDdKwJkDyl979qlAEDwYq6fHDjNARgcBTKSmQAgiyn2jynJasC0AwZUk8aRJFgUJCBqDymGDDgynshoh+CsQX5ck8lkAsH35QA1pF7VAKSh5XBlDFCt6eTdK042kgBfDHAn6kTdJ7C6j1A9DylWlFBqA1j2kyk4SP6rgjhllZlCmrB1kUlwFulskjCemfTBh+Lw6HBJLNmZDtpRQsjtSj4yFaAOKLhhoFLTCQLZ4MLLDt74FHTVA2gUJNBtAdCQEDArYqDK4vLpDxy+n4A4AiLpCkCZlqA5mi5EjdJz64BXh5BKainUmFnkIlkWkNlqADkcDHbmnknfm2kWbcDEnEhsRtgdgcBtmYivldnem9nAUQCgU3iti+KdjGHARwQIQACkXy0qKYxhfYlA6c0qnwJQWKhw9Aww42ZKUAKU9K6QaIxhv+8EDgOFB5EKR5NZUyfJLoTQDAIF7OJJAhKikQnsOEZQXepauIdEYAlIYA8CKiaF7YnYCYKgjF6Zl5F+15uZd5pA75xZ6QpZ1pFZQlYFqg/5LZQFIAWSapqempx65JLpbAsF+AHpIAwKCFPFIA/pYAgZIUG+8gCAXImOTBqpmMjl9OTs6mecolOIxhsS+Co2JMyuV5UyN5eZeQBZIIH5xlX5plUyKpapGpRgKgXguQbwHEtZBV5ZUyEg8cyy/EloTQHx0Gd8zlMF7p7J8FPZPlflAVwZ8KAkoV7C4V9lDIZV0V24uwcVuCCVbI55KV3AaVOlGVel+ZBluVRlJldVIADKEgkQzV7IbVg8FVWIYA1Vn6Tx+p+gTZWZPg/gQQIQ4QF47Kcw5w6ouB5MK5dEa5dQG5LQ7QJIz4UBdaO6epOQZA+gdp0pjpwg0yWg4GoeFgqApoaNOAoZb1kCeC6Cy5fBuM81/ZxwgIn1K+ooXoqAWy7wMYAY+yyQO5qAEIUIhhRo2+UChgRIqw1iqNVgoZlqWAo4Q0Hek4rWBNOCO09By4cwqc0qRhi4Yozk0o3A8oioOA9NOBmoP14tpQbNfY90Eqt89IbOMlTQ0aQGItE6aNGNoZQG7oKYGy3o00OyAYZsb8YYEY1NLtcY8KFcV45YcI7MOYDBLYsCYACIvN1gGNzEbEBhsIkdkdNk+C6MnIogdE9gzgB0cIkI+CSQfgaeukCo8VhwNKZACIlFgoaOGOlo74fk9oUgiYyYjKZtFt6hIsSs1UY6hAxcscjV5KtMwcQuqMKmSwAQfgp08gys1IF4Xc14Om9okAgkR4WgPk7AOZUUaMGMj4VCddi4OZE5bobOmQ0mw4+ACdNttw2BDCWt2oOtYA60/YNwvwpI1Cr6OIAKVknwTF8YWYDWiwZQNw+ByQgwnqjkFQ9CMgyQFecIYhn17gFek1QgnyAAVMgxOFbag/EIuPGfWuJKcJHaGVDOhLcHg1kJgP0AALyOAOA3BT6/2PJlDUMxolhMA0gljNAsQ0jg21jloAB+GxxcVQ3BWgFDWgQE1EIoi4kBcKPQ6SBDtIhwhCKhBmGOrQZCxZrKjWO28k1wVExc59VgzEM9iON4KUNBZQgdAkmQy9q93wCAZg7gkpLiNB8QJw7N0UxAhAwEtEw2EdGNkdAAPloEE0BgituEE14JEGisE4E34HEwE3E34Ak/E4E0jTvTzG+FlME2IyBOBGUEE9hc4Pk6k+bdw2UASGBSDVhHCkE1RLRAUzExjUE6U++KmlI+kzUzk0cQEPk7tHFMU001oKg0wANASKIJg0E0E6gwBM0dCBMwYzgBUmZPbBVqlFJE0K04YPSqifiGkGnSLLwz03RHnAMIQD1oIYcGzFmBzF5CcNzH4siK1s5EcFICaP4+84Y9oGGDbN2VHejXzVoLBtCI4VoBUiYtaDpjcO6pNM5MxtwMiPHiyI0DknsHPmQNkiKrcJrUwnffGGYOyBY8vvPhWES2QFPuGHAH4wC0CyC80IHFCyTBTOCHFnCwcmjszY2kGJVXJAIuWojIrsSIQBBMNhWEROGKCuoU2PmE3pNgMGWouMBFoHCFHDs46FoEK2o3SpLnORylS5jYC8C1oFZP/Q6G4nROYpWnuSKLfO/DqzLQwP/BsDpA/ZjvLfoTCAHVoTeayKJE1taAvHq6GTS1oEBmNkxRWAEjWMoEsLY+vepFBVg4YFY3clIBYgokWkpQbEy0zXFgUkHZzHcyfhkzgDTMsxZLs5aLpDtIc0dAADxnPyPBv0ki23xaMSRgBviVBQRXioB4hjX0BVZV6Lj2j20DBZswvTRu12S3RGweqlt2wWSqA/rmVEgQXoUwAlgGGi49AsCNAzwgGfZgFRabyM0+r4CgAVCsDpBWQSWhJcakDiWViSVEM4gEnIXCV5AwYODfqhaMsUYRbfa/bkawsJZJbnuhbug9A3tPt3v4DcYgCPsybpQvtQBvsoUklcY/sHuLxHu0bRb/YwEzyXtQe3tSSg4PukfIfoRocfswagSfoMekAhUVA6AhWPC7vsAzy3VseMBeBkB97GrQkmAuhsSNoEDAQ/tGoMgnmgBfAgjsAv4ODpbpygS+Z+ANEQCgR+ChCECRAQAWIMAOAIDqd8gwChCacIAxHXZfoMdAA= -->
<!-- internal state end -->
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ansible_base/rbac/api/views.py`:
- Around line 466-467: Apply distinct() to both obj_assignments and
global_assignments before the nested iteration in the fallback serializer path,
matching the existing distinct() behavior near the serializer’s earlier query
construction. Add a regression test where multiple permissions with the same
content type match one assignment, and verify serialization emits that
assignment only once.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f7d3398-be5f-49dc-9a0f-9da74b6cbb5d
📒 Files selected for processing (3)
ansible_base/rbac/api/serializers.pyansible_base/rbac/api/views.pytest_app/tests/rbac/api/test_access_lists.py
kdelee
force-pushed
the
kdelee/prefetch-user-access-assignments
branch
2 times, most recently
from
August 10, 2026 19:18
28fb0e4 to
71c7d93
Compare
kdelee
marked this pull request as draft
August 10, 2026 20:01
Member
Author
ScaleLab validation: 26-next (without fix) vs devel (with fix)Compared the same endpoints on scalelab-a between Gateway:
|
| page_size | 26-next (old) | devel (patched) | Speedup |
|---|---|---|---|
| 10 | 8.55s | 0.20s | 43x |
| 100 | 9.21s | 0.29s | 32x |
Controller: role_user_access/awx.jobtemplate
| 26-next | devel | |
|---|---|---|
| page_size=100 | 0.62s | 0.29s |
Controller: role_team_access/awx.jobtemplate
| 26-next | devel | |
|---|---|---|
| page_size=100 | 0.24s | 0.29s |
The gateway shared.organization endpoint shows the largest improvement because it has the most users (7,153) and was fully serializer-bound. Controller-side JT endpoint also improves but the baseline was lower since JTs have fewer users with access.
SLO probe comparison (from workload comparison script, 26-next with fix applied via custom image built for devel w/ latest DAB + these changes)
role_team_access: awx.credential [team_access_tab] | 0.175 0.235 0.248 0% | 0.194 0.375 0.487 0% | ~same
role_team_access: awx.inventory [team_access_tab] | 0.175 0.235 0.248 0% | 0.175 0.235 0.249 0% | ~same
role_team_access: awx.jobtemplate [team_access_tab] | 0.175 0.235 0.248 0% | 0.300 0.460 0.496 0% | WORSE
role_team_access: awx.project [team_access_tab] | 0.175 0.235 0.248 0% | 0.190 0.350 0.485 0% | ~same
role_team_access: shared.organization [team_access_tab] | 0.175 0.235 0.248 0% | 0.175 0.235 0.248 0% | ~same
role_user_access: awx.credential [user_access_tab] | 0.625 1.98 2.45 0% | 0.350 0.470 0.497 0% | BETTER
role_user_access: awx.inventory [user_access_tab] | 0.583 0.717 0.747 0% | 0.406 0.625 0.738 0% | BETTER
role_user_access: awx.jobtemplate [user_access_tab] | 0.575 0.735 0.960 0% | 0.338 0.468 0.497 0% | BETTER
role_user_access: awx.project [user_access_tab] | 0.587 0.738 2.28 0% | 0.385 0.492 0.715 0% | BETTER
role_user_access: shared.organization [user_access_tab] | 8.44 9.69 9.97 0% | 0.333 0.467 0.497 0% | BETTER
teams: org_member list [team_list_rbac] | 0.375 0.475 0.497 0% | 0.375 0.475 0.497 0% | ~same
The one showing "WORSE" I'd say is within margin of error, the p99 is still acceptable.
kdelee
marked this pull request as ready for review
August 10, 2026 20:14
kdelee
force-pushed
the
kdelee/prefetch-user-access-assignments
branch
2 times, most recently
from
August 10, 2026 20:20
8c0de22 to
56270c1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## devel #1096 +/- ##
==========================================
- Coverage 94.77% 94.71% -0.07%
==========================================
Files 258 258
Lines 14405 14450 +45
Branches 2202 2214 +12
==========================================
+ Hits 13653 13686 +33
- Misses 752 764 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
john-westcott-iv
requested changes
Aug 11, 2026
john-westcott-iv
left a comment
john-westcott-iv
left a comment
Member
There was a problem hiding this comment.
Great optimization — the profiling numbers speak for themselves (478x serialization speedup). A few structural suggestions before merging.
1. The list() override and context-passing could live in the serializer instead
Right now the optimization spans two files: the view builds the prefetch dict, passes it through context, and the serializer reads it. But DRF already gives us a hook — when many=True, it wraps the child serializer in a ListSerializer. The child's self.parent points to that ListSerializer, and self.parent.instance holds the page of objects. We can lazy-prefetch on the first get_object_role_assignments() call and cache on the parent.
This would eliminate the list() override, the get_serializer_context() override, and the _build_prefetched_assignments() view method entirely. Here's what it would look like in AccessListMixin:
def get_object_role_assignments(self, actor):
# --- Lazy prefetch for list serialization ---
# DRF note: when many=True, DRF wraps us in a ListSerializer.
# self.parent points to it, and self.parent.instance holds the
# page of objects. self.parent is None during __init__ but IS
# set by the time SerializerMethodField methods run (bind()
# happens inside to_representation).
if self.parent is not None:
if not hasattr(self.parent, '_prefetched_assignments'):
# DRF note: for paginated views, instance is a list (the page).
# For unpaginated views it could be a lazy queryset — iterating
# it triggers one extra DB query, which is still better than N+1.
actors = getattr(self.parent, 'instance', None)
if actors is not None:
self.parent._prefetched_assignments = (
self._bulk_fetch_assignments([a.pk for a in actors])
)
# DRF note: we're reading an attribute we set on the ListSerializer
# instance. This is safe because each request gets its own serializer
# tree (no instance reuse across requests, no thread-safety concern).
# It's not a documented DRF extension point, but the ListSerializer
# lifecycle has been stable across DRF versions for years.
prefetched = getattr(self.parent, '_prefetched_assignments', None)
if prefetched is not None:
return prefetched.get(actor.pk, [])
# Fallback: single-object serialization (existing path, unchanged)
obj = self.context.get("related_object")
permission = self.context.get("permission")
ct = self.context.get("content_type")
if permission:
assignment_qs = assignment_qs_user_to_obj_perm(actor, obj, permission)
else:
assignment_qs = assignment_qs_user_to_obj(actor, obj)
return self.summarize_assignment_list(assignment_qs, ct)
def _bulk_fetch_assignments(self, actor_ids):
"""Bulk-fetch and classify assignments for a page of actors."""
obj = self.context["related_object"]
permission = self.context.get("permission")
ct = self.context["content_type"]
actor_cls = self.Meta.model
assignment_cls = actor_cls._meta.get_field('role_assignments').related_model
actor_field = 'user_id' if actor_cls._meta.model_name == 'user' else 'team_id'
evaluation_cls = get_evaluation_model(obj)
eval_filter = dict(object_id=obj.pk, content_type_id=ct.id)
if permission:
eval_filter['codename'] = permission.codename
obj_eval_qs = evaluation_cls.objects.filter(**eval_filter)
obj_assignments = assignment_cls.objects.filter(
object_role__in=obj_eval_qs.values_list('role_id', flat=True),
**{f'{actor_field}__in': actor_ids},
).select_related('role_definition')
global_filter = {
'content_type': None,
f'{actor_field}__in': actor_ids,
}
if permission:
global_filter['role_definition__permissions'] = permission
else:
global_filter['role_definition__permissions__content_type'] = ct
global_assignments = assignment_cls.objects.filter(
**global_filter
).select_related('role_definition')
# Reuse the same classification logic as summarize_assignment_list
obj_ct = ct
team_ct = DABContentType.objects.get_for_model(get_team_model())
assignments_by_actor = {}
for qs_part in (obj_assignments, global_assignments):
for a in qs_part.distinct():
actor_id = getattr(a, actor_field)
if a.content_type_id is None:
perm_type = "global"
elif a.content_type_id == team_ct.pk:
perm_type = "team"
elif a.content_type_id == obj_ct.pk:
perm_type = "direct"
else:
perm_type = "indirect"
entry = {
"type": perm_type,
"role_definition": self.summarize_role_definition(a.role_definition),
}
assignments_by_actor.setdefault(actor_id, []).append(entry)
return assignments_by_actorWhat this eliminates:
- The
list()override in views.py - The
get_serializer_context()override - The
_build_prefetched_assignments()method on the view - The
get_url_for_objectimport added to views.py - The hardcoded
"_prefetched_assignments"context key sync problem
2. If option 1 feels too DRF-internal, a lighter fix for the current approach
If the self.parent approach feels like too much reliance on DRF internals, the current PR structure is workable — but please address:
- Use a constant for the context key —
"_prefetched_assignments"is hardcoded in bothviews.pyandserializers.py. If either drifts, the prefetch silently stops working and falls back to N+1 queries with zero errors. A shared constant (e.g._PREFETCHED_ASSIGNMENTS_KEYin a common location) prevents that. - Extract the classification logic —
_build_prefetched_assignments()in views.py reimplements the same global/team/direct/indirect classification thatsummarize_assignment_list()already does in serializers.py (lines 295-309). Either call the existing method, or extract the classification into a small shared helper that both paths use. Two copies of this logic will diverge eventually. - Squash commits — the second commit message is
"maybe was not using prefetch, try fix that"which should be folded in.
3. Lines 489-491 are correct
page = self.paginate_queryset(queryset)
data = page if page is not None else querysetThis mirrors DRF's own ListModelMixin.list() — paginate_queryset() returns None when no paginator is configured, in which case you serialize the full queryset. It looks odd but is standard DRF convention.
kdelee
force-pushed
the
kdelee/prefetch-user-access-assignments
branch
from
August 11, 2026 16:15
c42c5a1 to
7e45d24
Compare
Member
Author
|
Refactored using John's suggested change, results still look good note: the couple endpoitns that say "worse" are still within margin of error i'd say (less that 200ms) and in acceptable latency in general (p50 less than 500ms, p99 less than 1500 ms) |
bhavenst
approved these changes
Aug 11, 2026
bhavenst
left a comment
bhavenst
left a comment
Contributor
There was a problem hiding this comment.
John's review comments have been thoroughly addressed:
Option 1 fully adopted — all prefetch logic moved from the view into the serializer using the self.parent (ListSerializer) lazy-prefetch pattern. This eliminates the list() override, get_serializer_context() override, _build_prefetched_assignments() view method, and the cross-file context key sync problem.
Commits squashed into a single clean commit with a thorough message.
CodeRabbit .distinct() applied at the iteration level to prevent duplicates from multi-permission content type joins.
ScaleLab re-validated — updated SLO numbers confirm the refactored version maintains the performance gains (shared.organization user access: 7.73s → 0.175s p50).
Minor nit: the PR description summary still references get_serializer_context() which no longer exists after the refactor — worth a quick update for future readers, but not a blocker.
john-westcott-iv
approved these changes
Aug 11, 2026
john-westcott-iv
left a comment
john-westcott-iv
left a comment
Member
There was a problem hiding this comment.
Re-reviewed — all previous comments addressed.
- Prefetch moved to serializer —
_bulk_fetch_assignments()and lazy-prefetch logic now live entirely inAccessListMixinusing theself.parent(ListSerializer) pattern. Thelist()override,get_serializer_context()override, and view-level_build_prefetched_assignments()are all gone ✔ - No cross-file key sync needed —
_prefetched_assignmentsis now an attribute onself.parentwithin the same class, not a string passed through serializer context ✔ - Reuses
summarize_role_definition()for the role_definition dict ✔ - DRF notes explain the
self.parent/ ListSerializer mechanics ✔ - Commits squashed to a single clean commit ✔
- Bonus
select_related('resource')in the queryset avoids N+1 on theansible_idlookup ✔
LGTM.
kdelee
force-pushed
the
kdelee/prefetch-user-access-assignments
branch
from
August 11, 2026 17:17
7e45d24 to
e277a57
Compare
Member
DVT Report: AAP-86117 Prefetch object_role_assignments to eliminate N+1 serializer queriesPR: #1096 Summary
PR Description SummaryPerformance optimization that bulk-prefetches role assignments during access list serialization instead of running per-user N+1 queries. Reduces serialization from 4.5s to 0.009s (478x speedup) for pages with 100 users. Adds select_related('resource') to the queryset to avoid additional queries for the related field. JIRA Acceptance Criteria CoverageSource: AAP-86117 No acceptance criteria found on the JIRA ticket. Test Results1. User access list returns correct results ✅ (PASS)
curl commandcurl -sk -u admin:<pass> https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/2. Assignment types correctly classified (direct, global) ✅ (PASS)
curl commandcurl -sk -u admin:<pass> https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/3. User with multiple roles shows all assignments ✅ (PASS)
curl commandcurl -sk -u admin:<pass> https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/4. Superuser shows with empty assignments ✅ (PASS)
curl commandcurl -sk -u admin:<pass> https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/5. Global role (Platform Auditor) user in access list ✅ (PASS)
curl commandcurl -sk -u admin:<pass> https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/6. Team access list with team role assignment ✅ (PASS)
curl commandcurl -sk -u admin:<pass> https://localhost:8000/api/gateway/v1/role_team_access/shared.organization/1/7. Unauthenticated access returns 401 ✅ (PASS)
curl commandcurl -sk https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/8. Non-superuser can access user access list ✅ (PASS)
curl commandcurl -sk -u dvt_user:dvt_password_Str0ng! https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/9. Admin write token auth on access list ✅ (PASS)
curl commandcurl -sk -H 'Authorization: Bearer <admin_write_token>' https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/10. User read token auth on access list ✅ (PASS)
curl commandcurl -sk -H 'Authorization: Bearer <user_read_token>' https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/11. Pagination page_size=5 returns correct page ✅ (PASS)
curl commandcurl -sk -u admin:<pass> 'https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/?page_size=5'12. Page 2 returns correct assignments (separate prefetch) ✅ (PASS)
curl commandcurl -sk -u admin:<pass> 'https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/?page_size=5&page=2'13. Consistency: full page vs paginated assignments match ✅ (PASS)
curl commandMultiple requests: page_size=100 vs page_size=5 across 3 pages14. Role user assignment CRUD still works ✅ (PASS)
curl commandPOST /users/, POST /role_user_assignments/, GET /role_user_access/15. Related field uses ansible_id in details URL ✅ (PASS)
curl commandcurl -sk -u admin:<pass> https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/16. Details drill-down endpoint works ✅ (PASS)
curl commandcurl -sk -u admin:<pass> https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/<ansible_id>/17. Role definitions endpoint (regression) ✅ (PASS)
curl commandcurl -sk -u admin:<pass> https://localhost:8000/api/gateway/v1/role_definitions/18. Invalid content type returns 404 ✅ (PASS)
curl commandcurl -sk -u admin:<pass> https://localhost:8000/api/gateway/v1/role_user_access/invalid.type/1/19. Non-existent object ID returns 404 ✅ (PASS)
curl commandcurl -sk -u admin:<pass> https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/99999/20. Assignment deletion removes user from access list ✅ (PASS)
curl commandDELETE /role_user_assignments/13/, then GET /role_user_access/shared.organization/1/21. Prefetch handles empty assignment set (superuser) ✅ (PASS)
curl commandcurl -sk -u admin:<pass> https://localhost:8000/api/gateway/v1/role_user_access/shared.organization/1/Coverage Assessment21 tests cover all API-exercisable aspects of the PR: correct assignment type classification (direct, global), multi-role users, superuser handling, team access lists, pagination correctness, consistency between full and paginated views, auth variants (session, token), CRUD operations, and negative cases. The bulk prefetch optimization is validated indirectly through correctness — ensuring the prefetched results match the per-user query path. Query count and scalelab performance criteria require unit tests and infrastructure access respectively. Recommended Unit TestsEdge cases discovered during DVT that should be covered by unit tests:
MethodologyThis DVT was performed by an AI assistant against a live AAP Gateway instance Test categories:
|
…r queries
The user/team access list endpoints (/api/v1/role_user_access/,
/api/v1/role_team_access/) called get_object_role_assignments() per
actor on every page, issuing one assignment query per row. On an
organization with ~7k users this made the endpoint unusable (8-10s p50
under load).
Fix: lazy-prefetch all assignments for the page on the first serializer
call via self.parent (the DRF ListSerializer), then serve subsequent
actors from the cache. Also add select_related('resource') to eliminate
the per-actor resource registry lookup.
Query count is now constant regardless of page size:
page_size=5: 10 queries
page_size=25: 10 queries
page_size=100: 10 queries
ScaleLab validation (no load, 26-next vs devel with fix, Default org ~6.9k users):
GET /api/gateway/v1/role_user_access/shared.organization/1/?page_size=100
26-next: 0.61s devel: 0.19s (3.2x)
GET /api/controller/v2/role_user_access/shared.organization/322/?page_size=100
26-next: 1.23s devel: 0.42s (2.9x)
GET /api/controller/v2/role_user_access/awx.jobtemplate/33158/?page_size=100
26-next: 0.68s devel: 0.40s (1.7x)
Under-load SLO probe comparison (26-next vs devel, p50/p90/p99):
GET /api/controller/v2/role_user_access/shared.organization/<pk>/
8.75/9.75/9.97s -> 0.18/0.24/0.25s
GET /api/controller/v2/role_user_access/awx.credential/<pk>/
1.15/2.23/2.47s -> 0.38/0.48/0.50s
GET /api/controller/v2/role_user_access/awx.jobtemplate/<pk>/
0.58/0.72/0.75s -> 0.38/0.48/0.50s
GET /api/controller/v2/role_user_access/awx.inventory/<pk>/
0.61/0.72/0.75s -> 0.36/0.47/0.50s
GET /api/controller/v2/role_user_access/awx.project/<pk>/
0.54/0.74/2.32s -> 0.36/0.50/0.97s
GET /api/controller/v2/role_team_access/<model>/<pk>/
~same (no regression; small p50 increase is baseline DAB overhead
visible on all endpoints including unrelated ones)
Assisted-by: John Westcott @john-westcott-iv
Assisted-by: Claude Code via Google Vertex AI
kdelee
force-pushed
the
kdelee/prefetch-user-access-assignments
branch
from
August 11, 2026 19:46
5dbb0c6 to
83a987b
Compare
|
DVCS PR Check Results: PR appears valid (JIRA key(s) found) |
john-westcott-iv
enabled auto-merge (squash)
August 11, 2026 19:58
|
Member
|
I want to try and return to this at some point because it's very likely it introduces code duplication that we could pay down somehow, almost certainly. Otherwise, at a high-level glance it doesn't look bad particularly. |
This was referenced Aug 17, 2026
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UserAccessViewSet.get_serializer_context()with a single queryAccessListMixin.get_object_role_assignments()instead of hitting the DB per userUserAccessViewSetandTeamAccessViewSetbenefit (shared parent class)Context
Follow-up to #1092 (collect-IDs fix for the queryset). The queryset optimization brought the DB query from seconds to 21ms, but the endpoint was still 8-9 seconds. Profiling on scalelab gateway showed serialization was the bottleneck:
get_object_role_assignmentsis aSerializerMethodFieldthat callsassignment_qs_user_to_obj_perm(actor, obj, permission)per user on every pagepage_size=100, that's 100 DISTINCT queries at ~42ms each = 4.3s in serialization aloneProfiling results (scalelab gateway, Default org with 7,153 users)
Endpoint response time should drop from ~8s to under 1s for the
shared.organizationuser access tab.Test plan
test_user_access_listvalidates correctness ofobject_role_assignmentstypes (direct, indirect, team)test_user_access_list_query_countasserts that adding 18 users increases query count by fewer than 5 (would be 18+ without prefetch)_prefetched_assignmentsis not in context (backwards compatible)role_user_access: shared.organizationendpoint drops from ~8s to <1srole_user_accessandrole_team_accessendpointsSummary by CodeRabbit
Performance Improvements
Bug Fixes