Fixes using multiple serializers on derived viewsets

closes #3587
https://pulp.plan.io/issues/3587

pulpcore/pulpcore/app/serializers/base.py

@@ -232,8 +232,12 @@ def to_representation(self, instance):
         ret = OrderedDict()
 
         instance = instance.cast()
-        viewset = viewset_for_model(instance)
-        fields = viewset.serializer_class(context=self._context)._readable_fields
+        viewset = viewset_for_model(instance)()
+        if self.parent and self.parent.many:
+            viewset.action = 'list'
+        else:
+            viewset.action = 'get'
+        fields = viewset.get_serializer_class()(context=self._context)._readable_fields
 
         for field in fields:
             try:

pulpcore/pulpcore/app/viewsets/base.py

@@ -90,22 +90,25 @@ def get_serializer_class(self):
         defines a 'default' serializer, and can override that serializer on a per-action basis.
 
         e.g. serializers = {'default': TaskSerializer, 'list': MinimalTaskSerializer}
+
+        If you define both, the 'serializer_class' attribute will be ignored and the 'serializers'
+        attribute will be used. This is because plugin viewsets may want to use multiple
+        serializers, but they inherit from basic viewsets such as Content which will have a
+        'serializer_class' attribute defined.
         """
         serializer_class = getattr(self, 'serializer_class', None)
         serializers = getattr(self, 'serializers', None)
-        both = serializers and serializer_class
-        assert not both, _("{} defines both 'serializer_class' and 'serializers'. It should only "
-                           "define one.").format(self.__class__.__name__)
 
-        if serializer_class:
-            return serializer_class
+        msg = _("{} must either have a 'serializer_class' attribute, or it must have a "
+                "'serializers' attribute with a 'default' key set").format(self.__class__.__name__)
+        assert serializer_class or serializers, msg
 
-        valid = serializers and isinstance(serializers, dict) and serializers.get('default', None)
-        assert valid, _("{} must either have a 'serializer_class' attribute, or it must have a "
-                        "'serializers' attribute with a 'default' key set").\
-            format(self.__class__.__name__)
+        if serializers:
+            valid = isinstance(serializers, dict) and 'default' in serializers.keys()
+            assert valid, msg
+            return self.serializers.get(self.action, serializers['default'])
 
-        return self.serializers.get(self.action, self.serializers['default'])
+        return serializer_class
 
     @staticmethod
     def get_resource(uri, model):

pulpcore/tests/viewsets/test_base.py

@@ -126,28 +126,6 @@ def test_resource_with_field_error(self):
 
 
 class TestGetSerializerClass(TestCase):
-    def test_defining_both_fails(self):
-        """
-        Test that get_serializer_class() raises an AssertionError if you try to define both
-        'serializer_class' and 'serializers'.
-        """
-        class TestTaskViewSet(viewsets.NamedModelViewSet):
-            serializer_class = serializers.TaskSerializer
-            serializers = {'default': serializers.TaskSerializer}
-
-        with self.assertRaises(AssertionError):
-            TestTaskViewSet().get_serializer_class()
-
-    def test_defining_neither_fails(self):
-        """
-        Test that get_serializer_class() raises an AssertionError if you try to define neither
-        'serializer_class' nor 'serializers'.
-        """
-        class TestTaskViewSet(viewsets.NamedModelViewSet):
-            pass
-
-        with self.assertRaises(AssertionError):
-            TestTaskViewSet().get_serializer_class()
 
     def test_serializer_class(self):
         """
@@ -178,6 +156,30 @@ class TestTaskViewSet(viewsets.NamedModelViewSet):
         viewset.action = 'list'
         self.assertEquals(viewset.get_serializer_class(), serializers.MinimalTaskSerializer)
 
+    def test_defining_neither_fails(self):
+        """
+        Test that get_serializer_class() raises an AssertionError if you try to define neither
+        'serializer_class' nor 'serializers'.
+        """
+        class TestTaskViewSet(viewsets.NamedModelViewSet):
+            pass
+
+        with self.assertRaises(AssertionError):
+            TestTaskViewSet().get_serializer_class()
+
+    def test_defining_both_uses_dict(self):
+        """
+        Test that get_serializer_class() will use 'serializers' if both 'serializer_class' and
+        'serializers' are defined.
+        """
+        class TestTaskViewSet(viewsets.NamedModelViewSet):
+            serializer_class = serializers.MinimalTaskSerializer
+            serializers = {'default': serializers.TaskSerializer}
+
+        viewset = TestTaskViewSet()
+        viewset.action = 'list'
+        self.assertEquals(viewset.get_serializer_class(), serializers.TaskSerializer)
+
     def test_serializers_without_default_fails(self):
         """
         Tests that get_serializer_class() raises an AssertionError if you defined 'serializers'
@@ -189,6 +191,21 @@ class TestTaskViewSet(viewsets.NamedModelViewSet):
         with self.assertRaises(AssertionError):
             TestTaskViewSet().get_serializer_class()
 
+    def test_multiple_derived_viewsets(self):
+        """
+        Tests that get_serializer_class() works on a viewset derived from another viewset which
+        inherits from NamedModelViewSet.
+        """
+        class TestAlternateSerializer(serializers.ContentSerializer):
+            pass
+
+        class TestDerivedViewSet(viewsets.ContentViewSet):
+            serializers = {'default': TestAlternateSerializer}
+
+        viewset = TestDerivedViewSet()
+        viewset.action = 'list'
+        self.assertEquals(viewset.get_serializer_class(), TestAlternateSerializer)
+
 
 class TestGetParentFieldAndObject(TestCase):
     def test_no_parent_object(self):