diff --git a/ansible_base/activitystream/views.py b/ansible_base/activitystream/views.py index 127a806dc..324370cc9 100644 --- a/ansible_base/activitystream/views.py +++ b/ansible_base/activitystream/views.py @@ -37,7 +37,7 @@ class EntryReadOnlyViewSet(ReadOnlyModelViewSet, AnsibleBaseDjangoAppApiView): API endpoint that allows for read-only access to activity stream entries. """ - queryset = Entry.objects.all() + queryset = Entry.objects.prefetch_related('created_by', 'content_type').all() serializer_class = EntrySerializer filter_backends = calculate_filter_backends() ordering = ['-id'] diff --git a/ansible_base/rbac/api/views.py b/ansible_base/rbac/api/views.py index 51874ff98..a72bcca3b 100644 --- a/ansible_base/rbac/api/views.py +++ b/ansible_base/rbac/api/views.py @@ -63,11 +63,20 @@ class RoleMetadataView(AnsibleBaseDjangoAppApiView, GenericAPIView): permission_classes = try_add_oauth2_scope_permission([permissions.IsAuthenticated]) serializer_class = RoleMetadataSerializer + def __init__(self, *args, **kwargs): + self.permission_cache = {} + def dispatch(self, request, *args, **kwargs): # Warm cache to avoid hits to basically all types from serializer DABContentType.objects.get_for_models(*permission_registry.all_registered_models) return super().dispatch(request, *args, **kwargs) + def get_for_codename(self, codename): + if codename not in self.permission_cache: + for permission in permission_registry.permission_qs.all(): + self.permission_cache[permission.codename] = permission + return self.permission_cache[codename] + def get(self, request, format=None): data = OrderedDict() allowed_permissions = OrderedDict() @@ -84,7 +93,7 @@ def get(self, request, format=None): cls_repr = f"{get_resource_prefix(cls)}.{cls._meta.model_name}" allowed_permissions[cls_repr] = [] for codename in list_combine_values(permissions_allowed_for_role(cls)): - perm = permission_registry.permission_qs.get(codename=codename) + perm = self.get_for_codename(codename) ct = permission_registry.content_type_model.objects.get_for_id(perm.content_type_id) perm_repr = f"{get_resource_prefix(ct.model_class())}.{codename}" allowed_permissions[cls_repr].append(perm_repr) @@ -107,7 +116,7 @@ class RoleDefinitionViewSet(AnsibleBaseDjangoAppApiView, ModelViewSet): but can be assigned to users. """ - queryset = RoleDefinition.objects.prefetch_related('created_by', 'modified_by', 'content_type', 'permissions') + queryset = RoleDefinition.objects.prefetch_related('created_by', 'modified_by', 'content_type', 'permissions', 'resource') serializer_class = RoleDefinitionSerializer permission_classes = try_add_oauth2_scope_permission([RoleDefinitionPermissions]) @@ -216,7 +225,7 @@ class RoleTeamAssignmentViewSet(BaseAssignmentViewSet): """ serializer_class = RoleTeamAssignmentSerializer - prefetch_related = ('team',) + prefetch_related = ('team__resource',) filter_backends = BaseAssignmentViewSet.filter_backends + [ ansible_id_backend.TeamAnsibleIdAliasFilterBackend, ansible_id_backend.RoleAssignmentFilterBackend, @@ -236,7 +245,7 @@ class RoleUserAssignmentViewSet(BaseAssignmentViewSet): """ serializer_class = RoleUserAssignmentSerializer - prefetch_related = ('user',) + prefetch_related = ('user__resource',) filter_backends = BaseAssignmentViewSet.filter_backends + [ ansible_id_backend.UserAnsibleIdAliasFilterBackend, ansible_id_backend.RoleAssignmentFilterBackend, diff --git a/ansible_base/rbac/service_api/serializers.py b/ansible_base/rbac/service_api/serializers.py index 8335a9391..603f79c41 100644 --- a/ansible_base/rbac/service_api/serializers.py +++ b/ansible_base/rbac/service_api/serializers.py @@ -1,5 +1,4 @@ from crum import impersonate -from django.apps import apps from django.db import transaction from django.utils.translation import gettext_lazy as _ from rest_framework import serializers @@ -9,6 +8,49 @@ from ..remote import RemoteObject +class ObjectAnsibleIdField(serializers.Field): + """ + Field for object_ansible_id that supports both annotation optimization and fallback. + + For read operations: Uses annotation when available, falls back to manual lookup. + For write operations: Converts ansible_id to object_id for internal use. + """ + + def to_representation(self, obj): + """Get object_ansible_id, using annotation when available, falling back to manual lookup""" + # First try to use the annotation from the queryset (for optimized list operations) + if hasattr(obj, '_object_ansible_id_annotation') and obj._object_ansible_id_annotation: + return str(obj._object_ansible_id_annotation) + + # Fallback for cases where annotation is not available (creation, etc.) + if not obj.content_type_id or not obj.object_id: + return None + + content_object = obj.content_object + if isinstance(content_object, RemoteObject): + return None + if hasattr(content_object, 'resource'): + return str(content_object.resource.ansible_id) + return None + + def get_attribute(self, instance): + """Override to return the full instance instead of a specific attribute""" + return instance + + def to_internal_value(self, value): + """Convert object_ansible_id to object_id for internal use""" + if not value: + return None + + from ansible_base.resource_registry.models import Resource + + try: + resource = Resource.objects.get(ansible_id=value) + return resource.object_id + except Resource.DoesNotExist: + raise serializers.ValidationError("Resource with this ansible_id does not exist.") + + class DABContentTypeSerializer(serializers.ModelSerializer): parent_content_type = serializers.SlugRelatedField(read_only=True, slug_field='api_slug') @@ -25,28 +67,6 @@ class Meta: fields = ['api_slug', 'codename', 'content_type', 'name'] -class ObjectIDAnsibleIDField(serializers.Field): - "This is an ansible_id field intended to be used with source pointing to object_id, so, does conversion" - - def to_representation(self, value): - "The source for this field is object_id, which is ignored, use content_object instead" - assignment = getattr(self, "_this_assignment", None) - if not assignment: - return None - content_object = assignment.content_object - if isinstance(content_object, RemoteObject): - return None - if hasattr(content_object, 'resource'): - return str(content_object.resource.ansible_id) - return None - - def to_internal_value(self, value): - "Targeting object_id, this converts ansible_id into object_id" - resource_cls = apps.get_model('dab_resource_registry', 'Resource') - resource = resource_cls.objects.get(ansible_id=value) - return resource.object_id - - assignment_common_fields = ('created', 'created_by_ansible_id', 'object_id', 'object_ansible_id', 'content_type', 'role_definition') @@ -54,39 +74,29 @@ class BaseAssignmentSerializer(serializers.ModelSerializer): content_type = serializers.SlugRelatedField(read_only=True, slug_field='api_slug') role_definition = serializers.SlugRelatedField(slug_field='name', queryset=RoleDefinition.objects.all()) created_by_ansible_id = ActorAnsibleIdField(source='created_by', required=False, allow_null=True) - object_ansible_id = ObjectIDAnsibleIDField(source='object_id', required=False, allow_null=True) + object_ansible_id = ObjectAnsibleIdField(required=False, allow_null=True) object_id = serializers.CharField(allow_blank=True, required=False, allow_null=True) from_service = serializers.CharField(write_only=True) - def to_representation(self, instance): - # hack to surface content_object for ObjectIDAnsibleIDField - self.fields["object_ansible_id"]._this_assignment = instance - return super().to_representation(instance) - - def get_created_by_ansible_id(self, obj): - return str(obj.created_by.resource.ansible_id) - def validate(self, attrs): """The object_id vs ansible_id is the only dual-write case, where we have to accept either So this does the mutual validation to assure we have sufficient data. """ - has_oid = bool(self.initial_data.get('object_id')) - has_oaid = bool(self.initial_data.get('object_ansible_id')) - rd = attrs['role_definition'] + has_object_id = 'object_id' in attrs and attrs['object_id'] + has_object_ansible_id = 'object_ansible_id' in attrs and attrs['object_ansible_id'] + if rd.content_type_id: - if not self.partial and not has_oid and not has_oaid: + if not self.partial and not has_object_id and not has_object_ansible_id: raise serializers.ValidationError("You must provide either 'object_id' or 'object_ansible_id'.") - elif not has_oaid: - # need to remove blank and null fields or else it can overwrite the non-null non-blank field - 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'] else: - if has_oaid or has_oid: + if has_object_id or has_object_ansible_id: raise serializers.ValidationError("Can not provide either 'object_id' or 'object_ansible_id' for system role") - # NOTE: right now not enforcing the case you provide both, could check for consistency later - return super().validate(attrs) def find_existing_assignment(self, queryset): diff --git a/ansible_base/rbac/service_api/views.py b/ansible_base/rbac/service_api/views.py index f2294ff46..862657485 100644 --- a/ansible_base/rbac/service_api/views.py +++ b/ansible_base/rbac/service_api/views.py @@ -1,4 +1,5 @@ from django.db import transaction +from django.db.models import OuterRef, Subquery from rest_framework import permissions, status, viewsets from rest_framework.decorators import action from rest_framework.response import Response @@ -6,6 +7,7 @@ from ansible_base.lib.utils.views.django_app_api import AnsibleBaseDjangoAppApiView from ansible_base.lib.utils.views.permissions import try_add_oauth2_scope_permission +from ansible_base.resource_registry.models import Resource from ansible_base.resource_registry.views import HasResourceRegistryPermissions from ansible_base.rest_filters.rest_framework import ansible_id_backend @@ -109,10 +111,23 @@ def perform_destroy(self, instance): instance.role_definition.remove_global_permission(instance.actor) +def resource_ansible_id_expr(ct_field='content_type_id', oid_field='object_id'): + return Subquery( + Resource.objects.filter( + content_type_id=OuterRef(ct_field), + object_id=OuterRef(oid_field), + ).values( + 'ansible_id' + )[:1] + ) + + class ServiceRoleUserAssignmentViewSet(BaseSerivceRoleAssignmentViewSet): """List of user assignments for cross-service communication""" - queryset = RoleUserAssignment.objects.prefetch_related('user__resource', *prefetch_related) + queryset = RoleUserAssignment.objects.prefetch_related('user__resource__content_type', *prefetch_related).annotate( + _object_ansible_id_annotation=resource_ansible_id_expr() + ) serializer_class = service_serializers.ServiceRoleUserAssignmentSerializer filter_backends = AnsibleBaseDjangoAppApiView.filter_backends + [ ansible_id_backend.UserAnsibleIdAliasFilterBackend, @@ -131,7 +146,9 @@ def unassign(self, request): class ServiceRoleTeamAssignmentViewSet(BaseSerivceRoleAssignmentViewSet): """List of team role assignments for cross-service communication""" - queryset = RoleTeamAssignment.objects.prefetch_related('team__resource', *prefetch_related) + queryset = RoleTeamAssignment.objects.prefetch_related('team__resource__content_type', *prefetch_related).annotate( + _object_ansible_id_annotation=resource_ansible_id_expr() + ) serializer_class = service_serializers.ServiceRoleTeamAssignmentSerializer filter_backends = AnsibleBaseDjangoAppApiView.filter_backends + [ ansible_id_backend.TeamAnsibleIdAliasFilterBackend, diff --git a/ansible_base/resource_registry/models/resource.py b/ansible_base/resource_registry/models/resource.py index 1ffc58665..75d34a823 100644 --- a/ansible_base/resource_registry/models/resource.py +++ b/ansible_base/resource_registry/models/resource.py @@ -95,11 +95,11 @@ def summary_fields(self): @property def resource_type(self): - return resource_type_cache(self.content_type.pk).name + return resource_type_cache(self.content_type_id).name @property def resource_type_obj(self): - return resource_type_cache(self.content_type.pk) + return resource_type_cache(self.content_type_id) class Meta: unique_together = ('content_type', 'object_id') diff --git a/test_app/defaults.py b/test_app/defaults.py index 585448b93..331580b83 100644 --- a/test_app/defaults.py +++ b/test_app/defaults.py @@ -143,7 +143,7 @@ USE_TZ = True -DEMO_DATA_COUNTS = {'organization': 150, 'user': 379, 'team': 43} +DEMO_DATA_COUNTS = {'organization': 150, 'user': 379, 'team': 43, 'roledefinition': 100} ANSIBLE_BASE_TEAM_MODEL = 'test_app.Team' ANSIBLE_BASE_ORGANIZATION_MODEL = 'test_app.Organization' diff --git a/test_app/management/commands/create_demo_data.py b/test_app/management/commands/create_demo_data.py index 3b01e2e67..bbda3fc34 100644 --- a/test_app/management/commands/create_demo_data.py +++ b/test_app/management/commands/create_demo_data.py @@ -8,7 +8,7 @@ from ansible_base.authentication.models import Authenticator, AuthenticatorUser from ansible_base.oauth2_provider.models import OAuth2Application from ansible_base.rbac import permission_registry -from ansible_base.rbac.models import DABContentType, RoleDefinition +from ansible_base.rbac.models import DABContentType, DABPermission, RoleDefinition from test_app.models import EncryptionModel, InstanceGroup, Inventory, Organization, Team, User @@ -20,6 +20,9 @@ def create_large(self, data_counts): start = time.time() self.stdout.write('') self.stdout.write('About to create large demo data set. This will take a while.') + + # Create standard models first + created_org_ids = [] for cls in (Organization, Team, User): count = data_counts[cls._meta.model_name] for i in range(count): @@ -28,9 +31,74 @@ def create_large(self, data_counts): if cls is User: data = {'username': name} elif cls is Team: - data['organization_id'] = i + 1 # fudged, teams fewer than orgs - cls.objects.create(**data) + # Use actual created organization IDs, cycling through them + if created_org_ids: + data['organization_id'] = created_org_ids[i % len(created_org_ids)] + else: + raise ValueError("Teams cannot be created before organizations") + obj = cls.objects.create(**data) + # Collect organization IDs for team creation + if cls is Organization: + created_org_ids.append(obj.id) self.stdout.write(f'Created {count} {cls._meta.model_name}') + + # Create RoleDefinitions with permissions + if 'roledefinition' in data_counts: + rd_count = data_counts['roledefinition'] + org_ct = DABContentType.objects.get_for_model(Organization) + + for i in range(rd_count): + # Create some sample permissions for each role definition + perm1 = DABPermission.objects.create(name=f'Can view large role {i}', codename=f'view_large_role_{i}', content_type=org_ct) + perm2 = DABPermission.objects.create(name=f'Can edit large role {i}', codename=f'edit_large_role_{i}', content_type=org_ct) + + # Create role definition with Organization content type + rd = RoleDefinition.objects.create(name=f'Large Role Definition {i}', description=f'Large demo role definition {i}', content_type=org_ct) + + # Add permissions to the role definition + rd.permissions.add(perm1, perm2) + + self.stdout.write(f'Created {rd_count} role definitions with permissions') + + # Create permission assignments for users and teams + if created_org_ids and 'user' in data_counts and 'team' in data_counts: + # Get created users and teams + large_users = list(User.objects.filter(username__startswith='large_user_')) + large_teams = list(Team.objects.filter(name__startswith='large_team_')) + large_orgs = list(Organization.objects.filter(name__startswith='large_organization_')) + large_rds = list(RoleDefinition.objects.filter(name__startswith='Large Role Definition')) + + # Give over 25 permissions to users + user_permissions_given = 0 + for user in large_users: + for rd in large_rds: + for org in large_orgs: + rd.give_permission(user, org) + user_permissions_given += 1 + if user_permissions_given >= 25: + break + if user_permissions_given >= 25: + break + if user_permissions_given >= 25: + break + + # Give over 25 permissions to teams + team_permissions_given = 0 + for team in large_teams: + for rd in large_rds: + for org in large_orgs: + rd.give_permission(team, org) + team_permissions_given += 1 + if team_permissions_given >= 25: + break + if team_permissions_given >= 25: + break + if team_permissions_given >= 25: + break + + self.stdout.write(f'Assigned {user_permissions_given} permissions to users') + self.stdout.write(f'Assigned {team_permissions_given} permissions to teams') + self.stdout.write(f'Finished creating large demo data in {time.time() - start:.2f} seconds') def handle(self, *args, **kwargs): diff --git a/test_app/tests/rbac/remote/test_service_api.py b/test_app/tests/rbac/remote/test_service_api.py index 074a67323..ba7cf4d76 100644 --- a/test_app/tests/rbac/remote/test_service_api.py +++ b/test_app/tests/rbac/remote/test_service_api.py @@ -491,3 +491,76 @@ def test_serializer_allows_null_values_in_validation(self, admin_api_client, ran # Verify that created_by is None in validated_data when null is passed validated_data = serializer.validated_data assert 'created_by' not in validated_data or validated_data.get('created_by') is None + + +@pytest.mark.django_db +class TestValidationErrors: + """Test validation error cases in service API serializers""" + + def test_system_role_with_object_id_error(self, admin_api_client, rando): + """Test that providing object_id for system role raises validation error""" + from ansible_base.rbac.models import RoleDefinition + + # Get a system role (no content_type) + system_rd = RoleDefinition.objects.managed.sys_auditor + assert system_rd.content_type_id is None, "Should be a system role" + + url = get_relative_url('serviceuserassignment-assign') + data = { + "role_definition": system_rd.name, + "user_ansible_id": str(rando.resource.ansible_id), + "object_id": "12345", # This should cause error for system role + } + + response = admin_api_client.post(url, data=data) + assert response.status_code == 400, response.data + assert "Can not provide either 'object_id' or 'object_ansible_id' for system role" in str(response.data) + + def test_system_role_with_object_ansible_id_error(self, admin_api_client, rando, organization): + """Test that providing object_ansible_id for system role raises validation error""" + from ansible_base.rbac.models import RoleDefinition + + # Get a system role (no content_type) + system_rd = RoleDefinition.objects.managed.sys_auditor + assert system_rd.content_type_id is None, "Should be a system role" + + url = get_relative_url('serviceuserassignment-assign') + data = { + "role_definition": system_rd.name, + "user_ansible_id": str(rando.resource.ansible_id), + "object_ansible_id": str(organization.resource.ansible_id), # This should cause error for system role + } + + response = admin_api_client.post(url, data=data) + assert response.status_code == 400, response.data + assert "Can not provide either 'object_id' or 'object_ansible_id' for system role" in str(response.data) + + def test_object_role_without_valid_object_error(self, admin_api_client, rando, inv_rd): + """Test that object role without valid object raises validation error""" + url = get_relative_url('serviceuserassignment-assign') + data = { + "role_definition": inv_rd.name, + "user_ansible_id": str(rando.resource.ansible_id), + "object_id": "99999", # Non-existent inventory ID + } + + response = admin_api_client.post(url, data=data) + assert response.status_code == 400, response.data + # Check if the error is about object not existing + error_msg = str(response.data) + assert "does not exist" in error_msg.lower() + + def test_object_role_without_object_specified_error(self, admin_api_client, rando, inv_rd): + """Test that object role without object_id raises validation error""" + url = get_relative_url('serviceuserassignment-assign') + data = { + "role_definition": inv_rd.name, + "user_ansible_id": str(rando.resource.ansible_id), + # No object_id or object_ansible_id provided + } + + response = admin_api_client.post(url, data=data) + assert response.status_code == 400, response.data + # Check if the error is about missing object_id or object_ansible_id + error_msg = str(response.data) + assert "You must provide either 'object_id' or 'object_ansible_id'" in error_msg diff --git a/test_app/tests/resource_registry/test_summary_fields_performance.py b/test_app/tests/resource_registry/test_summary_fields_performance.py new file mode 100644 index 000000000..0bf104ffd --- /dev/null +++ b/test_app/tests/resource_registry/test_summary_fields_performance.py @@ -0,0 +1,476 @@ +# Generated by Claude Sonnet 4 + +import os +import time + +import pytest +from django.db import connection + +from ansible_base.rbac.models import DABContentType, DABPermission, RoleDefinition +from test_app.models import Organization, Team, User + + +class TestSummaryFieldsPerformance: + """ + Test that summary fields with prefetch_related don't cause N+1 query issues. + """ + + def test_summary_fields_with_prefetch_related_performance(self, db): + """ + Test that accessing summary fields on multiple organizations with + prefetch_related('resource') doesn't trigger additional queries after the first organization. + """ + # Create multiple organizations + orgs = [] + for i in range(5): + org = Organization.objects.create(name=f'Org {i}', description=f'Description for org {i}') + orgs.append(org) + + # Query organizations with prefetch_related + organizations = Organization.objects.prefetch_related('resource', 'modified_by', 'created_by').all() + + # Reset query count before the test + connection.queries_log.clear() + + # Access get_summary_fields for each organization + for i, org in enumerate(organizations): + queries_before = len(connection.queries_log) + org.get_summary_fields() + queries_after = len(connection.queries_log) + queries_for_this_org = queries_after - queries_before + + # With comprehensive prefetching (resource, modified_by, created_by), + # all items should trigger 0 queries for get_summary_fields() + assert queries_for_this_org == 0, ( + f"Organization {i} triggered {queries_for_this_org} queries. " f"With comprehensive prefetching, expected 0 queries for all items." + ) + + def test_summary_fields_detailed_query_analysis(self, db): + """ + More detailed test that tracks exactly when queries are executed. + """ + # Create organizations + orgs = [] + for i in range(3): + org = Organization.objects.create(name=f'Org {i}', description=f'Description for org {i}') + orgs.append(org) + + # Query with prefetch_related + organizations = Organization.objects.prefetch_related('resource', 'modified_by', 'created_by').all() + + connection.queries_log.clear() + + summary_fields_list = [] + query_counts = [] + + for i, org in enumerate(organizations): + queries_before = len(connection.queries_log) + summary_fields = org.get_summary_fields() + queries_after = len(connection.queries_log) + queries_for_this_org = queries_after - queries_before + + summary_fields_list.append(summary_fields) + query_counts.append(queries_for_this_org) + + # Check query counts per organization + for i, queries_for_this_org in enumerate(query_counts): + # With proper prefetch_related, all items should trigger 0 queries + assert queries_for_this_org == 0, ( + f"Organization {i} triggered {queries_for_this_org} queries. " + f"Query counts per org: {query_counts}. " + f"With prefetch_related('resource'), expected 0 queries for all items." + ) + + def test_summary_fields_with_additional_prefetches(self, db): + """ + Test with additional prefetches that might be needed for summary fields. + """ + # Create organizations + orgs = [] + for i in range(3): + org = Organization.objects.create(name=f'Org {i}', description=f'Description for org {i}') + orgs.append(org) + + # Query with minimal prefetches - internal caching handles deeper relationships + organizations = Organization.objects.prefetch_related('resource', 'modified_by', 'created_by').all() + + connection.queries_log.clear() + + # Access summary fields for each organization + for i, org in enumerate(organizations): + queries_before = len(connection.queries_log) + org.get_summary_fields() + queries_after = len(connection.queries_log) + queries_for_this_org = queries_after - queries_before + + # With minimal prefetching + internal caching, all items should trigger 0 queries + assert queries_for_this_org == 0, ( + f"Organization {i} triggered {queries_for_this_org} queries. " f"With minimal prefetching, expected 0 queries for all items." + ) + + @pytest.mark.django_db + def test_summary_fields_content_verification(self): + """ + Verify that summary fields actually contain the expected resource information. + """ + # Create an organization + org = Organization.objects.create(name='Test Org', description='Test Description') + + # Get summary fields + summary_fields = org.get_summary_fields() + + # Verify that resource information is included + assert 'resource' in summary_fields + + # The resource should have summary fields too + resource_summary = summary_fields['resource'] + assert 'ansible_id' in resource_summary + assert 'resource_type' in resource_summary + + +class TestRoleDefinitionSummaryFieldsPerformance: + """ + Test that RoleDefinition summary fields with prefetch_related don't cause N+1 query issues. + """ + + def get_roledefinition_count(self): + """ + Determine how many RoleDefinitions to create based on environment variables. + Uses same pattern as create_demo_data command. + """ + if os.environ.get('LARGE'): + # Use a substantial number for performance testing, but not as many as other models + # since RoleDefinitions are typically fewer in number but more complex + return 100 + else: + # Small number for regular tests + return 5 + + def test_roledefinition_summary_fields_with_prefetch_related_performance(self, db): + """ + Test that accessing summary fields on multiple RoleDefinitions with + prefetch_related('resource') doesn't trigger additional queries after the first RoleDefinition. + """ + # Create multiple role definitions with permissions + role_definitions = [] + count = self.get_roledefinition_count() + + # Create a content type for permissions + org_ct = DABContentType.objects.get_for_model(Organization) + + print(f"\nCreating {count} RoleDefinitions for performance testing...") + for i in range(count): + # Create some sample permissions + perm1 = DABPermission.objects.create(name=f'Can view role {i}', codename=f'view_role_{i}', content_type=org_ct) + perm2 = DABPermission.objects.create(name=f'Can edit role {i}', codename=f'edit_role_{i}', content_type=org_ct) + + # Create role definition + rd = RoleDefinition.objects.create(name=f'Role Definition {i}', description=f'Description for role definition {i}') + + # Add permissions to the role definition + rd.permissions.add(perm1, perm2) + role_definitions.append(rd) + + # Query role definitions with prefetch_related (including permissions) + role_definitions_qs = RoleDefinition.objects.prefetch_related('resource', 'modified_by', 'created_by', 'permissions').all() + + # Reset query count before the test + connection.queries_log.clear() + + # Access summary fields for each role definition with timing + start_time = time.time() + for i, rd in enumerate(role_definitions_qs): + queries_before = len(connection.queries_log) + rd.summary_fields() + queries_after = len(connection.queries_log) + queries_for_this_rd = queries_after - queries_before + + # With proper prefetch_related, all items should trigger 0 queries + assert queries_for_this_rd == 0, ( + f"RoleDefinition {i} triggered {queries_for_this_rd} queries. " f"With prefetch_related('resource'), expected 0 queries for all items." + ) + + elapsed_time = time.time() - start_time + print(f"Processed {count} RoleDefinitions in {elapsed_time:.3f} seconds ({count/elapsed_time:.1f} items/sec)") + + def test_roledefinition_summary_fields_with_additional_prefetches(self, db): + """ + Test RoleDefinitions with comprehensive prefetching including resource relationships. + """ + # Create role definitions with permissions + role_definitions = [] + count = self.get_roledefinition_count() + + # Create a content type for permissions + org_ct = DABContentType.objects.get_for_model(Organization) + + print(f"\nCreating {count} RoleDefinitions with additional prefetches...") + for i in range(count): + # Create sample permissions + perm1 = DABPermission.objects.create(name=f'Can manage role {i}', codename=f'manage_role_{i}', content_type=org_ct) + perm2 = DABPermission.objects.create(name=f'Can delete role {i}', codename=f'delete_role_{i}', content_type=org_ct) + perm3 = DABPermission.objects.create(name=f'Can assign role {i}', codename=f'assign_role_{i}', content_type=org_ct) + + # Create role definition + rd = RoleDefinition.objects.create(name=f'Role Definition {i}', description=f'Description for role definition {i}') + + # Add permissions to the role definition + rd.permissions.add(perm1, perm2, perm3) + role_definitions.append(rd) + + # Query with minimal prefetches - internal caching handles deeper relationships + role_definitions_qs = RoleDefinition.objects.prefetch_related('resource', 'modified_by', 'created_by', 'permissions').all() + + connection.queries_log.clear() + + # Access summary fields for each role definition + for i, rd in enumerate(role_definitions_qs): + queries_before = len(connection.queries_log) + rd.summary_fields() + queries_after = len(connection.queries_log) + queries_for_this_rd = queries_after - queries_before + + # With minimal prefetching + internal caching, all items should trigger 0 queries + assert queries_for_this_rd == 0, ( + f"RoleDefinition {i} triggered {queries_for_this_rd} queries. " f"With minimal prefetching, expected 0 queries for all items." + ) + + @pytest.mark.django_db + def test_roledefinition_summary_fields_content_verification(self): + """ + Verify that RoleDefinition summary fields contain the expected information. + """ + # Create a role definition + rd = RoleDefinition.objects.create(name='Test Role Definition', description='Test Description') + + # Get summary fields + summary_fields = rd.summary_fields() + + # Verify expected fields are present + assert 'id' in summary_fields + assert 'name' in summary_fields + assert 'description' in summary_fields + assert 'managed' in summary_fields + assert summary_fields['name'] == 'Test Role Definition' + assert summary_fields['description'] == 'Test Description' + + +class TestUserSummaryFieldsPerformance: + """ + Test that User summary fields with prefetch_related don't cause N+1 query issues. + """ + + def test_user_summary_fields_with_prefetch_related_performance(self, db): + """ + Test that accessing summary fields on multiple Users with + prefetch_related('resource') doesn't trigger additional queries after the first User. + """ + # Create multiple users + users = [] + for i in range(5): + user = User.objects.create(username=f'user{i}', email=f'user{i}@example.com') + users.append(user) + + # Query users with prefetch_related + users_qs = User.objects.prefetch_related('resource', 'modified_by', 'created_by').all() + + # Reset query count before the test + connection.queries_log.clear() + + # Access summary fields for each user + for i, user in enumerate(users_qs): + queries_before = len(connection.queries_log) + user.summary_fields() + queries_after = len(connection.queries_log) + queries_for_this_user = queries_after - queries_before + + # With proper prefetch_related, all items should trigger 0 queries + assert queries_for_this_user == 0, ( + f"User {i} triggered {queries_for_this_user} queries. " f"With prefetch_related('resource'), expected 0 queries for all items." + ) + + def test_user_summary_fields_with_additional_prefetches(self, db): + """ + Test Users with comprehensive prefetching including resource relationships. + """ + # Create users + users = [] + for i in range(3): + user = User.objects.create(username=f'user{i}', email=f'user{i}@example.com') + users.append(user) + + # Query with minimal prefetches - internal caching handles deeper relationships + users_qs = User.objects.prefetch_related('resource', 'modified_by', 'created_by').all() + + connection.queries_log.clear() + + # Access summary fields for each user + for i, user in enumerate(users_qs): + queries_before = len(connection.queries_log) + user.summary_fields() + queries_after = len(connection.queries_log) + queries_for_this_user = queries_after - queries_before + + # With minimal prefetching + internal caching, all items should trigger 0 queries + assert queries_for_this_user == 0, ( + f"User {i} triggered {queries_for_this_user} queries. " f"With minimal prefetching, expected 0 queries for all items." + ) + + +class TestTeamSummaryFieldsPerformance: + """ + Test that Team summary fields with prefetch_related don't cause N+1 query issues. + """ + + def test_team_summary_fields_with_prefetch_related_performance(self, db): + """ + Test that accessing summary fields on multiple Teams with + prefetch_related('resource') doesn't trigger additional queries after the first Team. + """ + # Create an organization first (required for teams) + org = Organization.objects.create(name='Test Org', description='Test Description') + + # Create multiple teams + teams = [] + for i in range(5): + team = Team.objects.create(name=f'Team {i}', organization=org) + teams.append(team) + + # Query teams with prefetch_related + teams_qs = Team.objects.prefetch_related('resource', 'organization', 'modified_by', 'created_by').all() + + # Reset query count before the test + connection.queries_log.clear() + + # Access summary fields for each team + for i, team in enumerate(teams_qs): + queries_before = len(connection.queries_log) + team.summary_fields() + queries_after = len(connection.queries_log) + queries_for_this_team = queries_after - queries_before + + # With proper prefetch_related, all items should trigger 0 queries + assert queries_for_this_team == 0, ( + f"Team {i} triggered {queries_for_this_team} queries. " f"With prefetch_related('resource'), expected 0 queries for all items." + ) + + def test_team_summary_fields_with_additional_prefetches(self, db): + """ + Test Teams with comprehensive prefetching including resource and organization relationships. + """ + # Create an organization first + org = Organization.objects.create(name='Test Org', description='Test Description') + + # Create teams + teams = [] + for i in range(3): + team = Team.objects.create(name=f'Team {i}', organization=org) + teams.append(team) + + # Query with minimal prefetches - internal caching handles deeper relationships + teams_qs = Team.objects.prefetch_related('resource', 'organization', 'modified_by', 'created_by').all() + + connection.queries_log.clear() + + # Access summary fields for each team + for i, team in enumerate(teams_qs): + queries_before = len(connection.queries_log) + team.summary_fields() + queries_after = len(connection.queries_log) + queries_for_this_team = queries_after - queries_before + + # With minimal prefetching + internal caching, all items should trigger 0 queries + assert queries_for_this_team == 0, ( + f"Team {i} triggered {queries_for_this_team} queries. " f"With minimal prefetching, expected 0 queries for all items." + ) + + @pytest.mark.django_db + def test_team_summary_fields_content_verification(self): + """ + Verify that Team summary fields contain the expected information. + """ + # Create an organization and team + org = Organization.objects.create(name='Test Org', description='Test Description') + team = Team.objects.create(name='Test Team', organization=org) + + # Get summary fields using get_summary_fields which includes related fields + summary_fields = team.get_summary_fields() + + # Verify expected fields are present + assert 'id' in summary_fields or hasattr(team, 'id') + # Verify organization information is included (if it's in get_summary_fields) + if 'organization' in summary_fields: + org_summary = summary_fields['organization'] + assert 'id' in org_summary + assert 'name' in org_summary + assert org_summary['name'] == 'Test Org' + + # Also test the basic summary_fields method + basic_summary = team.summary_fields() + assert 'id' in basic_summary + assert 'name' in basic_summary + assert basic_summary['name'] == 'Test Team' + + +class TestCombinedModelsSummaryFieldsPerformance: + """ + Test summary fields performance across multiple model types in the same test. + """ + + @pytest.mark.django_db + def test_mixed_models_summary_fields_performance(self): + """ + Test accessing summary fields on multiple different model types to ensure + consistent performance patterns across all resource registry models. + """ + # Create test data + org = Organization.objects.create(name='Test Org', description='Test Description') + + users = [] + for i in range(3): + user = User.objects.create(username=f'user{i}', email=f'user{i}@example.com') + users.append(user) + + teams = [] + for i in range(3): + team = Team.objects.create(name=f'Team {i}', organization=org) + teams.append(team) + + # Create role definitions with permissions + role_definitions = [] + org_ct = DABContentType.objects.get_for_model(Organization) + + # Use smaller count for combined test to keep it manageable + rd_count = 100 if os.environ.get('LARGE') else 3 + print(f"\nCreating {rd_count} RoleDefinitions for combined test...") + + for i in range(rd_count): + # Create permissions for this role definition + perm1 = DABPermission.objects.create(name=f'Can view resource {i}', codename=f'view_resource_{i}', content_type=org_ct) + perm2 = DABPermission.objects.create(name=f'Can edit resource {i}', codename=f'edit_resource_{i}', content_type=org_ct) + + rd = RoleDefinition.objects.create(name=f'Role Definition {i}', description=f'Description for role definition {i}') + rd.permissions.add(perm1, perm2) + role_definitions.append(rd) + + # Test each model type with minimal prefetching + models_and_queries = [ + (Organization.objects.prefetch_related('resource', 'modified_by', 'created_by'), 'Organization'), + (User.objects.prefetch_related('resource', 'modified_by', 'created_by'), 'User'), + (Team.objects.prefetch_related('resource', 'organization', 'modified_by', 'created_by'), 'Team'), + (RoleDefinition.objects.prefetch_related('resource', 'modified_by', 'created_by', 'permissions'), 'RoleDefinition'), + ] + + for queryset, model_name in models_and_queries: + connection.queries_log.clear() + + for i, instance in enumerate(queryset.all()): + queries_before = len(connection.queries_log) + instance.summary_fields() + queries_after = len(connection.queries_log) + queries_for_this_instance = queries_after - queries_before + + # With proper prefetching, all items should trigger 0 queries + assert queries_for_this_instance == 0, ( + f"{model_name} {i} triggered {queries_for_this_instance} queries. " f"With proper prefetching, expected 0 queries for all items." + ) diff --git a/test_app/tests/test_demo_data.py b/test_app/tests/test_demo_data.py index 80d970efd..fb30a3ba5 100644 --- a/test_app/tests/test_demo_data.py +++ b/test_app/tests/test_demo_data.py @@ -1,5 +1,9 @@ +import os +from unittest.mock import patch + import pytest +from ansible_base.rbac.models import RoleDefinition from test_app.management.commands.create_demo_data import Command from test_app.models import Organization @@ -24,3 +28,55 @@ def test_demo_data_idempotent(admin_user): assert Organization.objects.filter(name='AWX_community').exists() Command().handle() assert Organization.objects.filter(name='AWX_community').count() == 1 + + +@pytest.mark.django_db +@patch.dict(os.environ, {'LARGE': '1'}) +def test_demo_data_large_mode_creates_roledefinitions(admin_user): + """ + Test that when LARGE=1 environment variable is set, the create_demo_data command + creates a large number of RoleDefinitions with permissions as specified in settings. + """ + # Verify no large data exists initially + assert not Organization.objects.filter(name__startswith='large_').exists() + assert not RoleDefinition.objects.filter(name__startswith='Large Role Definition').exists() + + # Run the command with LARGE environment variable set + Command().handle() + + # Verify standard demo data was created + assert Organization.objects.filter(name='AWX_community').exists() + + # Verify large datasets were created as specified in DEMO_DATA_COUNTS + from django.conf import settings + + expected_rd_count = settings.DEMO_DATA_COUNTS['roledefinition'] + + # Check that the expected number of RoleDefinitions were created + large_role_definitions = RoleDefinition.objects.filter(name__startswith='Large Role Definition') + assert large_role_definitions.count() == expected_rd_count + + # Verify that RoleDefinitions have permissions attached + sample_rd = large_role_definitions.first() + assert sample_rd.permissions.count() == 2 # Each RD should have 2 permissions + + # Verify permission structure + permissions = sample_rd.permissions.all() + permission_codenames = [p.codename for p in permissions] + assert any('view_large_role_' in codename for codename in permission_codenames) + assert any('edit_large_role_' in codename for codename in permission_codenames) + + # Verify that RoleDefinitions have the correct content_type (Organization) + assert sample_rd.content_type is not None + assert sample_rd.content_type.model == 'organization' + + # Verify that over 25 permissions were assigned to users and teams + from ansible_base.rbac.models import RoleTeamAssignment, RoleUserAssignment + + # Count user permissions (RoleUserAssignment records) + user_assignments = RoleUserAssignment.objects.count() + assert user_assignments >= 25, f"Expected at least 25 user role assignments, got {user_assignments}" + + # Count team permissions (RoleTeamAssignment records) + team_assignments = RoleTeamAssignment.objects.count() + assert team_assignments >= 25, f"Expected at least 25 team role assignments, got {team_assignments}"