diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 6a94879a61..0693ca0c55 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -671,7 +671,7 @@ class UnifiedJobTemplateSerializer(BaseSerializer): else: return super(UnifiedJobTemplateSerializer, self).get_types() - def to_representation(self, obj): + def get_sub_serializer(self, obj): serializer_class = None if type(self) is UnifiedJobTemplateSerializer: if isinstance(obj, Project): @@ -684,6 +684,10 @@ class UnifiedJobTemplateSerializer(BaseSerializer): serializer_class = SystemJobTemplateSerializer elif isinstance(obj, WorkflowJobTemplate): serializer_class = WorkflowJobTemplateSerializer + return serializer_class + + def to_representation(self, obj): + serializer_class = self.get_sub_serializer(obj) if serializer_class: serializer = serializer_class(instance=obj, context=self.context) # preserve links for list view @@ -766,7 +770,7 @@ class UnifiedJobSerializer(BaseSerializer): return summary_fields - def to_representation(self, obj): + def get_sub_serializer(self, obj): serializer_class = None if type(self) is UnifiedJobSerializer: if isinstance(obj, ProjectUpdate): @@ -781,6 +785,10 @@ class UnifiedJobSerializer(BaseSerializer): serializer_class = SystemJobSerializer elif isinstance(obj, WorkflowJob): serializer_class = WorkflowJobSerializer + return serializer_class + + def to_representation(self, obj): + serializer_class = self.get_sub_serializer(obj) if serializer_class: serializer = serializer_class(instance=obj, context=self.context) # preserve links for list view @@ -818,7 +826,7 @@ class UnifiedJobListSerializer(UnifiedJobSerializer): else: return super(UnifiedJobListSerializer, self).get_types() - def to_representation(self, obj): + def get_sub_serializer(self, obj): serializer_class = None if type(self) is UnifiedJobListSerializer: if isinstance(obj, ProjectUpdate): @@ -832,7 +840,11 @@ class UnifiedJobListSerializer(UnifiedJobSerializer): elif isinstance(obj, SystemJob): serializer_class = SystemJobListSerializer elif isinstance(obj, WorkflowJob): - serializer_class = WorkflowJobSerializer + serializer_class = WorkflowJobListSerializer + return serializer_class + + def to_representation(self, obj): + serializer_class = self.get_sub_serializer(obj) if serializer_class: serializer = serializer_class(instance=obj, context=self.context) ret = serializer.to_representation(obj) @@ -1480,7 +1492,9 @@ class ProjectUpdateDetailSerializer(ProjectUpdateSerializer): class ProjectUpdateListSerializer(ProjectUpdateSerializer, UnifiedJobListSerializer): - pass + class Meta: + model = ProjectUpdate + fields = ('*', '-controller_node') # field removal undone by UJ serializer class ProjectUpdateCancelSerializer(ProjectUpdateSerializer): @@ -2200,7 +2214,9 @@ class InventoryUpdateSerializer(UnifiedJobSerializer, InventorySourceOptionsSeri class InventoryUpdateListSerializer(InventoryUpdateSerializer, UnifiedJobListSerializer): - pass + class Meta: + model = InventoryUpdate + fields = ('*', '-controller_node') # field removal undone by UJ serializer class InventoryUpdateCancelSerializer(InventoryUpdateSerializer): @@ -3547,12 +3563,6 @@ class WorkflowJobTemplateSerializer(JobTemplateMixin, LabelsListMixin, UnifiedJo return vars_validate_or_raise(value) -# TODO: -class WorkflowJobTemplateListSerializer(WorkflowJobTemplateSerializer): - pass - - -# TODO: class WorkflowJobSerializer(LabelsListMixin, UnifiedJobSerializer): class Meta: @@ -3583,7 +3593,6 @@ class WorkflowJobSerializer(LabelsListMixin, UnifiedJobSerializer): return ret -# TODO: class WorkflowJobListSerializer(WorkflowJobSerializer, UnifiedJobListSerializer): class Meta: @@ -3866,7 +3875,10 @@ class AdHocCommandListSerializer(AdHocCommandSerializer, UnifiedJobListSerialize class SystemJobListSerializer(SystemJobSerializer, UnifiedJobListSerializer): - pass + + class Meta: + model = SystemJob + fields = ('*', '-controller_node') # field removal undone by UJ serializer class JobHostSummarySerializer(BaseSerializer): diff --git a/awx/api/views.py b/awx/api/views.py index c6cb9cb0c0..79c268f666 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -642,7 +642,7 @@ class InstanceUnifiedJobsList(SubListAPIView): view_name = _("Instance Jobs") model = UnifiedJob - serializer_class = UnifiedJobSerializer + serializer_class = UnifiedJobListSerializer parent_model = Instance def get_queryset(self): @@ -689,7 +689,7 @@ class InstanceGroupUnifiedJobsList(SubListAPIView): view_name = _("Instance Group Running Jobs") model = UnifiedJob - serializer_class = UnifiedJobSerializer + serializer_class = UnifiedJobListSerializer parent_model = InstanceGroup relationship = "unifiedjob_set" @@ -800,7 +800,7 @@ class ScheduleCredentialsList(LaunchConfigCredentialsBase): class ScheduleUnifiedJobsList(SubListAPIView): model = UnifiedJob - serializer_class = UnifiedJobSerializer + serializer_class = UnifiedJobListSerializer parent_model = Schedule relationship = 'unifiedjob_set' view_name = _('Schedule Jobs List') @@ -1058,7 +1058,7 @@ class OrganizationProjectsList(SubListCreateAttachDetachAPIView): class OrganizationWorkflowJobTemplatesList(SubListCreateAttachDetachAPIView): model = WorkflowJobTemplate - serializer_class = WorkflowJobTemplateListSerializer + serializer_class = WorkflowJobTemplateSerializer parent_model = Organization relationship = 'workflows' parent_key = 'organization' @@ -1380,7 +1380,7 @@ class ProjectNotificationTemplatesSuccessList(SubListCreateAttachDetachAPIView): class ProjectUpdatesList(SubListAPIView): model = ProjectUpdate - serializer_class = ProjectUpdateSerializer + serializer_class = ProjectUpdateListSerializer parent_model = Project relationship = 'project_updates' @@ -1491,7 +1491,7 @@ class ProjectUpdateScmInventoryUpdates(SubListCreateAPIView): view_name = _("Project Update SCM Inventory Updates") model = InventoryUpdate - serializer_class = InventoryUpdateSerializer + serializer_class = InventoryUpdateListSerializer parent_model = ProjectUpdate relationship = 'scm_inventory_updates' parent_key = 'source_project_update' @@ -2830,7 +2830,7 @@ class InventorySourceGroupsList(SubListDestroyAPIView): class InventorySourceUpdatesList(SubListAPIView): model = InventoryUpdate - serializer_class = InventoryUpdateSerializer + serializer_class = InventoryUpdateListSerializer parent_model = InventorySource relationship = 'inventory_updates' @@ -3701,7 +3701,7 @@ class WorkflowJobNodeAlwaysNodesList(WorkflowJobNodeChildrenBaseList): class WorkflowJobTemplateList(WorkflowsEnforcementMixin, ListCreateAPIView): model = WorkflowJobTemplate - serializer_class = WorkflowJobTemplateListSerializer + serializer_class = WorkflowJobTemplateSerializer always_allow_superuser = False diff --git a/awx/main/tests/unit/api/serializers/test_unified_serializers.py b/awx/main/tests/unit/api/serializers/test_unified_serializers.py new file mode 100644 index 0000000000..a03ceb7d7a --- /dev/null +++ b/awx/main/tests/unit/api/serializers/test_unified_serializers.py @@ -0,0 +1,69 @@ +# AWX +from awx.api import serializers +from awx.main.models import UnifiedJob, UnifiedJobTemplate + +# DRF +from rest_framework.generics import ListAPIView + + +def test_unified_template_field_consistency(): + ''' + Example of what is being tested: + The endpoints /projects/N/ and /projects/ should have the same fields as + that same project when it is serialized by the unified job template serializer + in /unified_job_templates/ + ''' + for cls in UnifiedJobTemplate.__subclasses__(): + detail_serializer = getattr(serializers, '{}Serializer'.format(cls.__name__)) + unified_serializer = serializers.UnifiedJobTemplateSerializer().get_sub_serializer(cls()) + assert set(detail_serializer().fields.keys()) == set(unified_serializer().fields.keys()) + + +def test_unified_job_list_field_consistency(): + ''' + Example of what is being tested: + The endpoint /project_updates/ should have the same fields as that + project update when it is serialized by the unified job template serializer + in /unified_jobs/ + ''' + for cls in UnifiedJob.__subclasses__(): + list_serializer = getattr(serializers, '{}ListSerializer'.format(cls.__name__)) + unified_serializer = serializers.UnifiedJobListSerializer().get_sub_serializer(cls()) + assert set(list_serializer().fields.keys()) == set(unified_serializer().fields.keys()), ( + 'Mismatch between {} list serializer & unified list serializer'.format(cls) + ) + + +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') + ) + for cls in UnifiedJob.__subclasses__(): + list_serializer = getattr(serializers, '{}ListSerializer'.format(cls.__name__)) + detail_serializer = getattr(serializers, '{}Serializer'.format(cls.__name__)) + list_fields = set(list_serializer().fields.keys()) + detail_fields = set(detail_serializer().fields.keys()) - allowed_detail_fields + assert list_fields == detail_fields, 'List / detail mismatch for serializers of {}'.format(cls) + + +def test_list_views_use_list_serializers(all_views): + ''' + Check that the list serializers are only used for list views, + and vice versa + ''' + list_serializers = tuple( + getattr(serializers, '{}ListSerializer'.format(cls.__name__)) for + cls in (UnifiedJob.__subclasses__() + [UnifiedJob]) + ) + for View in all_views: + if hasattr(View, 'model') and issubclass(getattr(View, 'model'), UnifiedJob): + if issubclass(View, ListAPIView): + assert issubclass(View.serializer_class, list_serializers), ( + 'View {} serializer {} is not a list serializer'.format(View, View.serializer_class) + ) + else: + assert not issubclass(View.model, list_serializers) diff --git a/awx/main/tests/unit/conftest.py b/awx/main/tests/unit/conftest.py index 2307b3a47d..cb73ee8c55 100644 --- a/awx/main/tests/unit/conftest.py +++ b/awx/main/tests/unit/conftest.py @@ -3,6 +3,11 @@ import logging from mock import PropertyMock +from awx.api.urls import urlpatterns as api_patterns + +# Django +from django.core.urlresolvers import RegexURLResolver, RegexURLPattern + @pytest.fixture(autouse=True) def _disable_database_settings(mocker): @@ -10,6 +15,33 @@ def _disable_database_settings(mocker): m.return_value = [] +@pytest.fixture() +def all_views(): + ''' + returns a set of all views in the app + ''' + patterns = set([]) + url_views = set([]) + # Add recursive URL patterns + unprocessed = set(api_patterns) + while unprocessed: + to_process = unprocessed.copy() + unprocessed = set([]) + for pattern in to_process: + if hasattr(pattern, 'lookup_str') and not pattern.lookup_str.startswith('awx.api'): + continue + patterns.add(pattern) + if isinstance(pattern, RegexURLResolver): + for sub_pattern in pattern.url_patterns: + if sub_pattern not in patterns: + unprocessed.add(sub_pattern) + # Get view classes + for pattern in patterns: + if isinstance(pattern, RegexURLPattern) and hasattr(pattern.callback, 'view_class'): + url_views.add(pattern.callback.view_class) + return url_views + + @pytest.fixture() def dummy_log_record(): return logging.LogRecord( diff --git a/awx/main/tests/unit/test_views.py b/awx/main/tests/unit/test_views.py index f1bad79400..060d979f76 100644 --- a/awx/main/tests/unit/test_views.py +++ b/awx/main/tests/unit/test_views.py @@ -5,9 +5,6 @@ import mock from rest_framework import exceptions from rest_framework.generics import ListAPIView -# Django -from django.core.urlresolvers import RegexURLResolver, RegexURLPattern - # AWX from awx.main.views import ApiErrorView from awx.api.views import JobList, InventorySourceList @@ -58,33 +55,12 @@ def test_disable_post_on_v1_inventory_source_list(version, supports_post): assert ('POST' in inv_source_list.allowed_methods) == supports_post -def test_views_have_search_fields(): - from awx.api.urls import urlpatterns as api_patterns - patterns = set([]) - url_views = set([]) - # Add recursive URL patterns - unprocessed = set(api_patterns) - while unprocessed: - to_process = unprocessed.copy() - unprocessed = set([]) - for pattern in to_process: - if hasattr(pattern, 'lookup_str') and not pattern.lookup_str.startswith('awx.api'): - continue - patterns.add(pattern) - if isinstance(pattern, RegexURLResolver): - for sub_pattern in pattern.url_patterns: - if sub_pattern not in patterns: - unprocessed.add(sub_pattern) - # Get view classes - for pattern in patterns: - if isinstance(pattern, RegexURLPattern) and hasattr(pattern.callback, 'view_class'): - cls = pattern.callback.view_class - if issubclass(cls, ListAPIView): - url_views.add(pattern.callback.view_class) - +def test_views_have_search_fields(all_views): # Gather any views that don't have search fields defined views_missing_search = [] - for View in url_views: + for View in all_views: + if not issubclass(View, ListAPIView): + continue view = View() if not hasattr(view, 'search_fields') or len(view.search_fields) == 0: views_missing_search.append(view)