Upstream16461 - remove extra_vars from jobs and unified_jobs list endpoint (#584)

* Upstream16461 - remove extra_vars from jobs and unified_jobs list endpoint

* Add a defer for the JobList to save a little DB I/O on extra_vars/artifacts
Doesn't really affect the UI, as all the screens use the Unified, but the api endpoint is used by the collection, so its a minor help.

Author: cigamit

awx/api/serializers.py

@@ -896,14 +896,37 @@ def get_launched_by(self, obj):
 
 
 class UnifiedJobListSerializer(UnifiedJobSerializer):
+    # Heavy fields that can hold large amounts of data. They are stripped from
+    # list responses by default to keep payloads small, and can be requested
+    # back explicitly (and only these) via the ?include=<field>[,<field>] query
+    # param handled by UnifiedJobIncludeMixin.
+    OPTIONAL_INCLUDE_FIELDS = frozenset({'artifacts', 'extra_vars'})
+
+    _ALWAYS_STRIPPED_FIELDS = frozenset({'job_args', 'job_cwd', 'job_env', 'result_traceback', 'event_processing_finished'})
+
     class Meta:
-        fields = ('*', '-job_args', '-job_cwd', '-job_env', '-result_traceback', '-event_processing_finished', '-artifacts')
+        fields = ('*', '-job_args', '-job_cwd', '-job_env', '-result_traceback', '-event_processing_finished')
+
+    @classmethod
+    def parse_requested_includes(cls, raw):
+        # Single source of truth for parsing the ?include= query param, shared
+        # with UnifiedJobIncludeMixin so the view-level defer and the
+        # serializer-level field stripping never drift apart.
+        requested = {name.strip() for name in (raw or '').split(',') if name.strip()}
+        return frozenset(requested) & cls.OPTIONAL_INCLUDE_FIELDS
+
+    def _requested_includes(self):
+        request = self.context.get('request')
+        if request is None:
+            return frozenset()
+        return self.parse_requested_includes(request.query_params.get('include', ''))
 
     def get_field_names(self, declared_fields, info):
         field_names = super(UnifiedJobListSerializer, self).get_field_names(declared_fields, info)
         # Meta multiple inheritance and -field_name options don't seem to be
         # taking effect above, so remove the undesired fields here.
-        return tuple(x for x in field_names if x not in ('job_args', 'job_cwd', 'job_env', 'result_traceback', 'event_processing_finished', 'artifacts'))
+        strip = self._ALWAYS_STRIPPED_FIELDS | (self.OPTIONAL_INCLUDE_FIELDS - self._requested_includes())
+        return tuple(x for x in field_names if x not in strip)
 
     def get_types(self):
         if type(self) is UnifiedJobListSerializer:

awx/api/views/__init__.py

@@ -118,6 +118,7 @@
     RelatedJobsPreventDeleteMixin,
     UnifiedJobDeletionMixin,
     NoTruncateMixin,
+    UnifiedJobIncludeMixin,
 )
 from awx.api.pagination import UnifiedJobEventPagination
 from awx.main.utils import set_environ
@@ -3499,10 +3500,23 @@ class SystemJobTemplateNotificationTemplatesSuccessList(SystemJobTemplateNotific
     relationship = 'notification_templates_success'
 
 
-class JobList(ListAPIView):
+class JobList(UnifiedJobIncludeMixin, ListAPIView):
     model = models.Job
     serializer_class = serializers.JobListSerializer
 
+    def get_queryset(self):
+        qs = super().get_queryset()
+        # extra_vars/artifacts are large columns that JobListSerializer omits
+        # from list responses unless requested via ?include=. Defer the ones
+        # that won't be serialized so the DB doesn't read their (potentially
+        # TOAST-ed) values. Only safe here because Job is queried directly
+        # (unlike the polymorphic UnifiedJobList).
+        requested = self.serializer_class.parse_requested_includes(self.request.query_params.get('include', ''))
+        to_defer = self.serializer_class.OPTIONAL_INCLUDE_FIELDS - requested
+        if to_defer:
+            qs = qs.defer(*sorted(to_defer))
+        return qs
+
 
 class JobDetail(UnifiedJobDeletionMixin, RetrieveDestroyAPIView):
     model = models.Job
@@ -4150,7 +4164,7 @@ class UnifiedJobTemplateList(ListAPIView):
     search_fields = ('description', 'name', 'jobtemplate__playbook')
 
 
-class UnifiedJobList(ListAPIView):
+class UnifiedJobList(UnifiedJobIncludeMixin, ListAPIView):
     model = models.UnifiedJob
     serializer_class = serializers.UnifiedJobListSerializer
     search_fields = ('description', 'name', 'job__playbook')

awx/api/views/mixin.py

@@ -216,3 +216,9 @@ def get_serializer_context(self):
         if self.request.query_params.get('no_truncate'):
             context.update(no_truncate=True)
         return context
+
+
+class UnifiedJobIncludeMixin(object):
+    # Reserve the name 'include' so we can use it as a query param. Otherwise, the rest-filters backend
+    # would treat it as a model field lookup.
+    rest_filters_reserved_names = ('include',)

awx/main/tests/functional/api/test_unified_jobs_view.py

@@ -145,3 +145,165 @@ def test_delete_ad_hoc_command_in_active_state(ad_hoc_command_factory, delete, a
     adhoc = ad_hoc_command_factory(initial_state=status)
     url = reverse('api:ad_hoc_command_detail', kwargs={'pk': adhoc.pk})
     delete(url, None, admin, expect=403)
+
+
+@pytest.fixture
+def job_with_heavy_fields(job_factory):
+    job = job_factory()
+    job.extra_vars = '{"some_var": "some_value"}'
+    job.artifacts = {"some_artifact": "some_value"}
+    job.save()
+    return job
+
+
+def _job_result(response, job_id):
+    for row in response.data['results']:
+        if row['id'] == job_id:
+            return row
+    raise AssertionError('job {} not found in {}'.format(job_id, [r['id'] for r in response.data['results']]))
+
+
+@pytest.mark.django_db
+def test_unified_jobs_list_excludes_heavy_fields_by_default(get, admin, job_with_heavy_fields):
+    response = get(reverse('api:unified_job_list') + '?id={}'.format(job_with_heavy_fields.id), admin, expect=200)
+    row = _job_result(response, job_with_heavy_fields.id)
+    assert 'artifacts' not in row
+    assert 'extra_vars' not in row
+
+
+@pytest.mark.django_db
+def test_unified_jobs_list_include_artifacts(get, admin, job_with_heavy_fields):
+    response = get(
+        reverse('api:unified_job_list') + '?id={}&include=artifacts'.format(job_with_heavy_fields.id),
+        admin,
+        expect=200,
+    )
+    row = _job_result(response, job_with_heavy_fields.id)
+    assert 'artifacts' in row
+    assert 'extra_vars' not in row
+
+
+@pytest.mark.django_db
+def test_unified_jobs_list_include_extra_vars(get, admin, job_with_heavy_fields):
+    response = get(
+        reverse('api:unified_job_list') + '?id={}&include=extra_vars'.format(job_with_heavy_fields.id),
+        admin,
+        expect=200,
+    )
+    row = _job_result(response, job_with_heavy_fields.id)
+    assert 'extra_vars' in row
+    assert 'artifacts' not in row
+
+
+@pytest.mark.django_db
+def test_unified_jobs_list_include_both(get, admin, job_with_heavy_fields):
+    response = get(
+        reverse('api:unified_job_list') + '?id={}&include=artifacts,extra_vars'.format(job_with_heavy_fields.id),
+        admin,
+        expect=200,
+    )
+    row = _job_result(response, job_with_heavy_fields.id)
+    assert 'artifacts' in row
+    assert 'extra_vars' in row
+
+
+@pytest.mark.django_db
+def test_unified_jobs_list_include_tolerates_whitespace(get, admin, job_with_heavy_fields):
+    response = get(
+        reverse('api:unified_job_list') + '?id={}&include=%20artifacts%20,%20extra_vars%20'.format(job_with_heavy_fields.id),
+        admin,
+        expect=200,
+    )
+    row = _job_result(response, job_with_heavy_fields.id)
+    assert 'artifacts' in row
+    assert 'extra_vars' in row
+
+
+@pytest.mark.django_db
+def test_unified_jobs_list_include_ignores_unknown(get, admin, job_with_heavy_fields):
+    response = get(
+        reverse('api:unified_job_list') + '?id={}&include=does_not_exist'.format(job_with_heavy_fields.id),
+        admin,
+        expect=200,
+    )
+    row = _job_result(response, job_with_heavy_fields.id)
+    assert 'artifacts' not in row
+    assert 'extra_vars' not in row
+
+
+@pytest.mark.django_db
+def test_unified_jobs_list_include_does_not_honor_always_stripped(get, admin, job_with_heavy_fields):
+    # Always-stripped fields like event_processing_finished, job_args, result_traceback
+    # must remain stripped regardless of the ?include= param — they cannot be re-included.
+    response = get(
+        reverse('api:unified_job_list') + '?id={}&include=job_args,result_traceback,event_processing_finished'.format(job_with_heavy_fields.id),
+        admin,
+        expect=200,
+    )
+    row = _job_result(response, job_with_heavy_fields.id)
+    assert 'event_processing_finished' not in row
+    assert 'job_args' not in row
+    assert 'result_traceback' not in row
+
+
+@pytest.mark.django_db
+def test_jobs_list_excludes_heavy_fields_by_default(get, admin, job_with_heavy_fields):
+    response = get(reverse('api:job_list') + '?id={}'.format(job_with_heavy_fields.id), admin, expect=200)
+    row = _job_result(response, job_with_heavy_fields.id)
+    assert 'artifacts' not in row
+    assert 'extra_vars' not in row
+
+
+@pytest.mark.django_db
+def test_jobs_list_include_extra_vars(get, admin, job_with_heavy_fields):
+    response = get(
+        reverse('api:job_list') + '?id={}&include=extra_vars'.format(job_with_heavy_fields.id),
+        admin,
+        expect=200,
+    )
+    row = _job_result(response, job_with_heavy_fields.id)
+    assert 'extra_vars' in row
+    assert 'artifacts' not in row
+
+
+def _deferred_field_names(qs):
+    # queryset.query.deferred_loading is (names, defer_flag). When defer_flag is
+    # True the names are deferred; when False (an .only() was applied) the
+    # deferred set is every concrete field not in names.
+    names, defer = qs.query.deferred_loading
+    if defer:
+        return set(names)
+    all_fields = {f.name for f in qs.model._meta.concrete_fields}
+    return all_fields - set(names)
+
+
+def _job_list_queryset(user, querystring=''):
+    from rest_framework.request import Request
+    from rest_framework.test import APIRequestFactory
+
+    from awx.api.views import JobList
+
+    view = JobList()
+    view.request = Request(APIRequestFactory().get('/api/v2/jobs/' + querystring))
+    view.request.user = user
+    return view.get_queryset()
+
+
+@pytest.mark.django_db
+def test_jobs_list_queryset_defers_heavy_columns_by_default(admin):
+    deferred = _deferred_field_names(_job_list_queryset(admin))
+    assert {'extra_vars', 'artifacts'} <= deferred
+
+
+@pytest.mark.django_db
+def test_jobs_list_queryset_defers_only_unrequested_heavy_columns(admin):
+    deferred = _deferred_field_names(_job_list_queryset(admin, '?include=extra_vars'))
+    assert 'artifacts' in deferred
+    assert 'extra_vars' not in deferred
+
+
+@pytest.mark.django_db
+def test_jobs_list_queryset_defers_nothing_when_all_included(admin):
+    deferred = _deferred_field_names(_job_list_queryset(admin, '?include=extra_vars,artifacts'))
+    assert 'extra_vars' not in deferred
+    assert 'artifacts' not in deferred

awx/main/tests/unit/api/serializers/test_unified_serializers.py

@@ -39,7 +39,9 @@ def test_unified_job_detail_exclusive_fields():
     For each type, assert that the only fields allowed to be exclusive to
     detail view are the allowed types
     """
-    allowed_detail_fields = frozenset(('result_traceback', 'job_args', 'job_cwd', 'job_env', 'event_processing_finished', 'artifacts'))
+    allowed_detail_fields = frozenset(
+        ('result_traceback', 'job_args', 'job_cwd', 'job_env', 'event_processing_finished', 'artifacts', 'extra_vars', 'context_message')
+    )
     for cls in UnifiedJob.__subclasses__():
         list_serializer = getattr(serializers, '{}ListSerializer'.format(cls.__name__))
         detail_serializer = getattr(serializers, '{}Serializer'.format(cls.__name__))