diff --git a/awx/api/pagination.py b/awx/api/pagination.py index 95f7b585b4..156a6c5ab1 100644 --- a/awx/api/pagination.py +++ b/awx/api/pagination.py @@ -6,12 +6,15 @@ from collections import OrderedDict # Django REST Framework from django.conf import settings from django.core.paginator import Paginator as DjangoPaginator +from django.utils.functional import cached_property from rest_framework import pagination from rest_framework.response import Response from rest_framework.utils.urls import replace_query_param from rest_framework.settings import api_settings from django.utils.translation import gettext_lazy as _ +from awx.main.models import ActivityStream + class DisabledPaginator(DjangoPaginator): @property @@ -23,6 +26,19 @@ class DisabledPaginator(DjangoPaginator): return 200 +class ActivityStreamPaginator(DjangoPaginator): + """Use unfiltered table count for activity stream pagination (AAP-83773). + + The RBAC-filtered COUNT query takes ~36 min on large tables due to the + pk__in subquery shape from AAP-81860. An unfiltered count is acceptable + for pagination UI -- an approximate over-count is harmless. + """ + + @cached_property + def count(self): + return ActivityStream.objects.count() + + class Pagination(pagination.PageNumberPagination): page_size_query_param = 'page_size' max_page_size = settings.MAX_PAGE_SIZE @@ -57,12 +73,13 @@ class Pagination(pagination.PageNumberPagination): def paginate_queryset(self, queryset, request, **kwargs): self.count_disabled = 'count_disabled' in request.query_params + original_paginator = self.django_paginator_class try: if self.count_disabled: self.django_paginator_class = DisabledPaginator return super(Pagination, self).paginate_queryset(queryset, request, **kwargs) finally: - self.django_paginator_class = DjangoPaginator + self.django_paginator_class = original_paginator def get_paginated_response(self, data): if self.count_disabled: @@ -70,6 +87,10 @@ class Pagination(pagination.PageNumberPagination): return super(Pagination, self).get_paginated_response(data) +class ActivityStreamPagination(Pagination): + django_paginator_class = ActivityStreamPaginator + + class LimitPagination(pagination.BasePagination): default_limit = api_settings.PAGE_SIZE limit_query_param = 'limit' diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index 47b6288959..85d3ae9d00 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -129,7 +129,7 @@ from awx.api.views.mixin import ( NoTruncateMixin, UnifiedJobExcludeMixin, ) -from awx.api.pagination import UnifiedJobEventPagination +from awx.api.pagination import ActivityStreamPagination, UnifiedJobEventPagination from awx.main.utils import set_environ logger = logging.getLogger('awx.api.views') @@ -4818,6 +4818,7 @@ class ActivityStreamList(SimpleListAPIView): model = models.ActivityStream serializer_class = serializers.ActivityStreamSerializer search_fields = ('changes',) + pagination_class = ActivityStreamPagination resource_purpose = 'audit trail entries for tracking system changes' @extend_schema_if_available( diff --git a/awx/main/tests/functional/api/test_activity_streams.py b/awx/main/tests/functional/api/test_activity_streams.py index e66276b3cb..2d35373a34 100644 --- a/awx/main/tests/functional/api/test_activity_streams.py +++ b/awx/main/tests/functional/api/test_activity_streams.py @@ -139,6 +139,29 @@ def test_stream_queryset_hides_shows_items( assert access.can_read(activity_stream_entry) +@pytest.mark.django_db +def test_activity_stream_pagination_uses_unfiltered_count(get, organization, project, user, settings): + """The pagination count should reflect total activity stream rows, not + the RBAC-filtered subset. The RBAC-filtered COUNT is catastrophically + slow on large tables (AAP-83773); an approximate over-count from an + unfiltered SELECT COUNT(*) is acceptable for pagination UI.""" + settings.ACTIVITY_STREAM_ENABLED = True + + no_access_user = user('no-access-user', False) + + total_entries = ActivityStream.objects.count() + assert total_entries > 0 + + url = reverse('api:activity_stream_list') + response = get(url, no_access_user) + + assert response.status_code == 200 + visible_results = len(response.data['results']) + pagination_count = response.data['count'] + assert pagination_count == total_entries + assert visible_results < pagination_count + + @pytest.mark.django_db def test_stream_user_direct_role_updates(get, post, organization_factory): objects = organization_factory('test_org', superusers=['admin'], users=['test'], inventories=['inv1']) diff --git a/awx/main/tests/unit/api/test_pagination.py b/awx/main/tests/unit/api/test_pagination.py new file mode 100644 index 0000000000..978c9e60df --- /dev/null +++ b/awx/main/tests/unit/api/test_pagination.py @@ -0,0 +1,63 @@ +from unittest.mock import patch, MagicMock + +from awx.api.pagination import ActivityStreamPaginator, ActivityStreamPagination, DisabledPaginator + + +class TestActivityStreamPaginator: + def test_count_uses_unfiltered_table_count(self): + with patch('awx.api.pagination.ActivityStream') as mock_as: + mock_as.objects.count.return_value = 713000 + paginator = ActivityStreamPaginator(object_list=[], per_page=25) + assert paginator.count == 713000 + mock_as.objects.count.assert_called_once() + + def test_count_is_cached(self): + with patch('awx.api.pagination.ActivityStream') as mock_as: + mock_as.objects.count.return_value = 500 + paginator = ActivityStreamPaginator(object_list=[], per_page=25) + _ = paginator.count + _ = paginator.count + mock_as.objects.count.assert_called_once() + + +class TestActivityStreamPagination: + def test_default_paginator_class(self): + pagination = ActivityStreamPagination() + assert pagination.django_paginator_class is ActivityStreamPaginator + + def test_normal_request_preserves_activity_stream_paginator(self): + pagination = ActivityStreamPagination() + request = MagicMock() + request.query_params = {} + + with patch('rest_framework.pagination.PageNumberPagination.paginate_queryset', return_value=[]): + pagination.paginate_queryset(MagicMock(), request) + + assert pagination.count_disabled is False + assert pagination.django_paginator_class is ActivityStreamPaginator + + def test_count_disabled_restores_activity_stream_paginator(self): + pagination = ActivityStreamPagination() + request = MagicMock() + request.query_params = {'count_disabled': 'true'} + + with patch('rest_framework.pagination.PageNumberPagination.paginate_queryset', return_value=[]): + pagination.paginate_queryset(MagicMock(), request) + + assert pagination.count_disabled is True + assert pagination.django_paginator_class is ActivityStreamPaginator + + def test_count_disabled_temporarily_uses_disabled_paginator(self): + pagination = ActivityStreamPagination() + request = MagicMock() + request.query_params = {'count_disabled': 'true'} + captured_class = {} + + def capture_paginator_class(self_inner, queryset, request, **kwargs): + captured_class['during'] = pagination.django_paginator_class + + with patch('rest_framework.pagination.PageNumberPagination.paginate_queryset', capture_paginator_class): + pagination.paginate_queryset(MagicMock(), request) + + assert captured_class['during'] is DisabledPaginator + assert pagination.django_paginator_class is ActivityStreamPaginator