Skip to content

Conversation

AlanCoding
Copy link
Member

@AlanCoding AlanCoding commented Sep 15, 2025

Description

This does general, obligatory, optimizations of API endpoints that were recently added.

The largest code change is because using LARGE=true with the bootstrap script wasn't creating all the RBAC objects, additional logic to do so was added. This did help to test cases of >25 Role definitions, role-user-assignments, and role-team-assignments.

This work is basically never-ending, so any stopping point is arbitrary. However, the new /service-index/ views greatly benefit from this.

The found solution often repeated for resource__content_type also suggests that the resource summary_fields entry is generally not well-optimized, because it's unlikely someone would have continued that to include __content_type.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Test update
  • Refactoring (no functional changes)
  • Development environment change
  • Configuration change

Note

Optimizes RBAC/service APIs with prefetch/annotation and cached permission lookups, refactors assignment serialization for ansible_id handling, and expands LARGE demo data with roles/permissions plus comprehensive tests.

  • RBAC/Service API performance:
    • Cache permission lookups in RoleMetadataView and pre-warm content types.
    • Add prefetches to role/assignment views: roledefinition queryset includes resource; team/user assignment viewsets use team__resource/user__resource and prefetch resource__content_type.
    • Annotate assignments with _object_ansible_id_annotation via subquery (resource_ansible_id_expr) to avoid N+1.
  • Serializers:
    • Replace ObjectIDAnsibleIDField with ObjectAnsibleIdField supporting annotation-based reads and ansible_id→object_id conversion; simplify BaseAssignmentSerializer validation/flow for object_id vs object_ansible_id.
  • Activity Stream:
    • Prefetch created_by and content_type in EntryReadOnlyViewSet.
  • Resource Registry:
    • Minor optimization using content_type_id in Resource.resource_type(_obj).
  • Demo data (LARGE mode):
    • Add roledefinition to DEMO_DATA_COUNTS; generate many RoleDefinitions with DABPermissions and assign >25 permissions to users/teams; fix team org assignment to use created org IDs.
  • Tests:
    • Add validation error cases for service assignment endpoints.
    • Add N+1/performance tests for summary_fields across Organization, User, Team, and RoleDefinition.
    • Extend demo data tests to validate LARGE mode role creation and assignments.

Written by Cursor Bugbot for commit 9d6746a. This will update automatically on new commits. Configure here.

@AlanCoding AlanCoding marked this pull request as ready for review September 17, 2025 18:24
@AlanCoding
Copy link
Member Author

In terms of the code coverage, that's low because of added test_app to create demo data. I don't want this counted in code coverage, at all.

@AlanCoding AlanCoding changed the title AAP-53611 General API optimizations for RBAC AAP-53278 General API optimizations for RBAC Oct 6, 2025
Copy link

github-actions bot commented Oct 6, 2025

DVCS PR Check Results:

PR appears valid (JIRA key(s) found)

Copy link

sonarqubecloud bot commented Oct 6, 2025

attrs['object_id'] = self.initial_data['object_id']
# If object_ansible_id was provided and converted, use that for object_id
if has_object_ansible_id and not has_object_id:
attrs['object_id'] = attrs['object_ansible_id']
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Field Validation Error Causes Semantic Confusion

The ObjectAnsibleIdField converts the input ansible_id to an object_id, storing it in attrs['object_ansible_id']. The validate method then incorrectly uses this attrs['object_ansible_id'] (now an object_id) to determine if an object_ansible_id was provided, and redundantly assigns it to attrs['object_id']. This creates semantic confusion and may lead to incorrect validation.

Fix in Cursor Fix in Web

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

Successfully merging this pull request may close these issues.

1 participant