mirror of
https://github.com/ansible/awx.git
synced 2026-10-31 06:26:15 -02:30
[AAP-93974] perf(api): optimize non-superuser job list pagination (#16668)
perf(api): optimize non-superuser job list pagination Co-authored-by: Jake Jackson <24478650+thedoubl3j@users.noreply.github.com>
This commit is contained in:
co-authored by
Jake Jackson
parent
33a9eb7d15
commit
f8314ada72
@@ -45,6 +45,15 @@ class UnifiedJobPaginator(DjangoPaginator):
|
|||||||
return UnifiedJob.objects.count()
|
return UnifiedJob.objects.count()
|
||||||
|
|
||||||
|
|
||||||
|
class JobPaginator(DjangoPaginator):
|
||||||
|
"""Count visible jobs through a primary-key subquery instead of full rows."""
|
||||||
|
|
||||||
|
@cached_property
|
||||||
|
def count(self):
|
||||||
|
visible_pks = self.object_list.order_by().values("pk")
|
||||||
|
return self.object_list.model.objects.filter(pk__in=visible_pks).count()
|
||||||
|
|
||||||
|
|
||||||
class Pagination(pagination.PageNumberPagination):
|
class Pagination(pagination.PageNumberPagination):
|
||||||
page_size_query_param = "page_size"
|
page_size_query_param = "page_size"
|
||||||
max_page_size = settings.MAX_PAGE_SIZE
|
max_page_size = settings.MAX_PAGE_SIZE
|
||||||
@@ -135,6 +144,10 @@ class UnifiedJobPagination(Pagination):
|
|||||||
return DjangoPaginator
|
return DjangoPaginator
|
||||||
|
|
||||||
|
|
||||||
|
class JobPagination(Pagination):
|
||||||
|
django_paginator_class = JobPaginator
|
||||||
|
|
||||||
|
|
||||||
class LimitPagination(pagination.BasePagination):
|
class LimitPagination(pagination.BasePagination):
|
||||||
default_limit = api_settings.PAGE_SIZE
|
default_limit = api_settings.PAGE_SIZE
|
||||||
limit_query_param = "limit"
|
limit_query_param = "limit"
|
||||||
|
|||||||
@@ -130,7 +130,7 @@ from awx.api.views.mixin import (
|
|||||||
NoTruncateMixin,
|
NoTruncateMixin,
|
||||||
UnifiedJobExcludeMixin,
|
UnifiedJobExcludeMixin,
|
||||||
)
|
)
|
||||||
from awx.api.pagination import ActivityStreamPagination, UnifiedJobEventPagination, UnifiedJobPagination
|
from awx.api.pagination import ActivityStreamPagination, JobPagination, UnifiedJobEventPagination, UnifiedJobPagination
|
||||||
from awx.main.utils import set_environ
|
from awx.main.utils import set_environ
|
||||||
|
|
||||||
logger = logging.getLogger('awx.api.views')
|
logger = logging.getLogger('awx.api.views')
|
||||||
@@ -3885,6 +3885,7 @@ class SystemJobTemplateNotificationTemplatesSuccessList(SystemJobTemplateNotific
|
|||||||
class JobList(UnifiedJobExcludeMixin, ListAPIView):
|
class JobList(UnifiedJobExcludeMixin, ListAPIView):
|
||||||
model = models.Job
|
model = models.Job
|
||||||
serializer_class = serializers.JobListSerializer
|
serializer_class = serializers.JobListSerializer
|
||||||
|
pagination_class = JobPagination
|
||||||
resource_purpose = 'jobs'
|
resource_purpose = 'jobs'
|
||||||
|
|
||||||
@extend_schema_if_available(extensions={"x-ai-description": "A list of jobs."})
|
@extend_schema_if_available(extensions={"x-ai-description": "A list of jobs."})
|
||||||
|
|||||||
+1
-1
@@ -1680,7 +1680,7 @@ class JobAccess(BaseAccess):
|
|||||||
if not org_access_qs.exists():
|
if not org_access_qs.exists():
|
||||||
return qs.filter(job_template__in=JobTemplate.access_qs(self.user, 'view'))
|
return qs.filter(job_template__in=JobTemplate.access_qs(self.user, 'view'))
|
||||||
|
|
||||||
return qs.filter(Q(job_template__in=JobTemplate.access_qs(self.user, 'view')) | Q(organization__in=org_access_qs)).distinct()
|
return qs.filter(Q(job_template__in=JobTemplate.access_qs(self.user, 'view')) | Q(organization__in=org_access_qs))
|
||||||
|
|
||||||
def can_add(self, data, validate_license=True):
|
def can_add(self, data, validate_license=True):
|
||||||
raise NotImplementedError('Direct job creation not possible in v2 API')
|
raise NotImplementedError('Direct job creation not possible in v2 API')
|
||||||
|
|||||||
@@ -138,6 +138,41 @@ def test_unified_job_list_team_member_sees_team_granted_jobs(user, setup_managed
|
|||||||
assert job.pk in result_ids, f"Team member should see job {job.pk} via team-granted JT execute permission, but got result IDs: {result_ids}"
|
assert job.pk in result_ids, f"Team member should see job {job.pk} via team-granted JT execute permission, but got result IDs: {result_ids}"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.django_db
|
||||||
|
def test_job_list_non_superuser_count_matches_visible_jobs(user, setup_managed_roles, get):
|
||||||
|
"""The job list count must include only jobs visible to the requesting user."""
|
||||||
|
visible_org = Organization.objects.create(name='job-list-visible-org')
|
||||||
|
visible_inventory = visible_org.inventories.create(name='job-list-visible-inv')
|
||||||
|
visible_project = Project.objects.create(name='job-list-visible-project', organization=visible_org)
|
||||||
|
visible_jt = JobTemplate.objects.create(
|
||||||
|
name='job-list-visible-jt',
|
||||||
|
project=visible_project,
|
||||||
|
inventory=visible_inventory,
|
||||||
|
organization=visible_org,
|
||||||
|
)
|
||||||
|
visible_job = visible_jt.create_unified_job()
|
||||||
|
|
||||||
|
hidden_org = Organization.objects.create(name='job-list-hidden-org')
|
||||||
|
hidden_inventory = hidden_org.inventories.create(name='job-list-hidden-inv')
|
||||||
|
hidden_project = Project.objects.create(name='job-list-hidden-project', organization=hidden_org)
|
||||||
|
hidden_jt = JobTemplate.objects.create(
|
||||||
|
name='job-list-hidden-jt',
|
||||||
|
project=hidden_project,
|
||||||
|
inventory=hidden_inventory,
|
||||||
|
organization=hidden_org,
|
||||||
|
)
|
||||||
|
hidden_jt.create_unified_job()
|
||||||
|
|
||||||
|
org_admin = user('job-list-org-admin')
|
||||||
|
RoleDefinition.objects.get(name='Organization Admin').give_permission(org_admin, visible_org)
|
||||||
|
|
||||||
|
response = get(reverse('api:job_list'), org_admin)
|
||||||
|
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert response.data['count'] == 1
|
||||||
|
assert [result['id'] for result in response.data['results']] == [visible_job.pk]
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.django_db
|
@pytest.mark.django_db
|
||||||
def test_unified_job_list_rando_sees_nothing(rando, setup_managed_roles, get):
|
def test_unified_job_list_rando_sees_nothing(rando, setup_managed_roles, get):
|
||||||
"""Unprivileged user sees no unified jobs."""
|
"""Unprivileged user sees no unified jobs."""
|
||||||
|
|||||||
@@ -102,6 +102,14 @@ def test_org_auditor_sees_orphans(normal_job, org_auditor):
|
|||||||
assert access.can_read(normal_job)
|
assert access.can_read(normal_job)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.django_db
|
||||||
|
def test_job_access_queryset_does_not_add_distinct(normal_job, org_admin):
|
||||||
|
queryset = JobAccess(org_admin).filtered_queryset()
|
||||||
|
|
||||||
|
assert not queryset.query.distinct
|
||||||
|
assert queryset.count() == queryset.values('pk').distinct().count()
|
||||||
|
|
||||||
|
|
||||||
# Delete permissions testing
|
# Delete permissions testing
|
||||||
@pytest.mark.django_db
|
@pytest.mark.django_db
|
||||||
def test_JT_admin_delete_denied(normal_job, rando):
|
def test_JT_admin_delete_denied(normal_job, rando):
|
||||||
|
|||||||
@@ -1,6 +1,14 @@
|
|||||||
from unittest.mock import patch, MagicMock
|
from unittest.mock import patch, MagicMock
|
||||||
|
|
||||||
from awx.api.pagination import ActivityStreamPaginator, ActivityStreamPagination, UnifiedJobPaginator, UnifiedJobPagination, DisabledPaginator
|
from awx.api.pagination import (
|
||||||
|
ActivityStreamPaginator,
|
||||||
|
ActivityStreamPagination,
|
||||||
|
DisabledPaginator,
|
||||||
|
JobPaginator,
|
||||||
|
JobPagination,
|
||||||
|
UnifiedJobPaginator,
|
||||||
|
UnifiedJobPagination,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class TestActivityStreamPaginator:
|
class TestActivityStreamPaginator:
|
||||||
@@ -80,6 +88,37 @@ class TestUnifiedJobPaginator:
|
|||||||
mock_uj.objects.count.assert_called_once()
|
mock_uj.objects.count.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
class TestJobPaginator:
|
||||||
|
def test_count_uses_primary_key_subquery(self):
|
||||||
|
object_list = MagicMock()
|
||||||
|
object_list.order_by.return_value.values.return_value = "visible_pks"
|
||||||
|
object_list.model.objects.filter.return_value.count.return_value = 713
|
||||||
|
|
||||||
|
paginator = JobPaginator(object_list=object_list, per_page=25)
|
||||||
|
|
||||||
|
assert paginator.count == 713
|
||||||
|
object_list.order_by.assert_called_once_with()
|
||||||
|
object_list.order_by.return_value.values.assert_called_once_with("pk")
|
||||||
|
object_list.model.objects.filter.assert_called_once_with(pk__in="visible_pks")
|
||||||
|
|
||||||
|
def test_count_is_cached(self):
|
||||||
|
object_list = MagicMock()
|
||||||
|
object_list.order_by.return_value.values.return_value = "visible_pks"
|
||||||
|
object_list.model.objects.filter.return_value.count.return_value = 500
|
||||||
|
paginator = JobPaginator(object_list=object_list, per_page=25)
|
||||||
|
|
||||||
|
_ = paginator.count
|
||||||
|
_ = paginator.count
|
||||||
|
|
||||||
|
object_list.model.objects.filter.assert_called_once_with(pk__in="visible_pks")
|
||||||
|
|
||||||
|
|
||||||
|
class TestJobPagination:
|
||||||
|
def test_default_paginator_class(self):
|
||||||
|
pagination = JobPagination()
|
||||||
|
assert pagination.django_paginator_class is JobPaginator
|
||||||
|
|
||||||
|
|
||||||
class TestUnifiedJobPagination:
|
class TestUnifiedJobPagination:
|
||||||
def test_default_paginator_class(self):
|
def test_default_paginator_class(self):
|
||||||
pagination = UnifiedJobPagination()
|
pagination = UnifiedJobPagination()
|
||||||
|
|||||||
Reference in New Issue
Block a user