Merge pull request #1582 from rca/12756-missing-yaml-module-serializer-error-message

Fixed #12756: Improved error message when yaml module is missing.

AUTHORS

@@ -58,6 +58,7 @@ answer newbie questions, and generally made Django that much better:
     Gisle Aas <gisle@aas.no>
     Chris Adams
     Mathieu Agopian <mathieu.agopian@gmail.com>
+    Roberto Aguilar <roberto@baremetal.io>
     ajs <adi@sieker.info>
     alang@bright-green.com
     A S Alam <aalam@users.sf.net>

django/core/management/commands/dumpdata.py

@@ -106,11 +106,11 @@ class Command(BaseCommand):
         # Check that the serialization format exists; this is a shortcut to
         # avoid collating all the objects and _then_ failing.
         if format not in serializers.get_public_serializer_formats():
-            raise CommandError("Unknown serialization format: %s" % format)
+            try:
+                serializers.get_serializer(format)
+            except serializers.SerializerDoesNotExist:
+                pass
 
-        try:
-            serializers.get_serializer(format)
-        except KeyError:
             raise CommandError("Unknown serialization format: %s" % format)
 
         def get_objects():

django/core/serializers/__init__.py

@@ -27,17 +27,29 @@ BUILTIN_SERIALIZERS = {
     "xml"    : "django.core.serializers.xml_serializer",
     "python" : "django.core.serializers.python",
     "json"   : "django.core.serializers.json",
+    "yaml"   : "django.core.serializers.pyyaml",
 }
 
-# Check for PyYaml and register the serializer if it's available.
-try:
-    import yaml
-    BUILTIN_SERIALIZERS["yaml"] = "django.core.serializers.pyyaml"
-except ImportError:
-    pass
-
 _serializers = {}
 
+
+class BadSerializer(object):
+    """
+    Stub serializer to hold exception raised during registration
+
+    This allows the serializer registration to cache serializers and if there
+    is an error raised in the process of creating a serializer it will be
+    raised and passed along to the caller when the serializer is used.
+    """
+    internal_use_only = False
+
+    def __init__(self, exception):
+        self.exception = exception
+
+    def __call__(self, *args, **kwargs):
+        raise self.exception
+
+
 def register_serializer(format, serializer_module, serializers=None):
     """Register a new serializer.
 
@@ -53,12 +65,23 @@ def register_serializer(format, serializer_module, serializers=None):
     """
     if serializers is None and not _serializers:
         _load_serializers()
-    module = importlib.import_module(serializer_module)
+
+    try:
+        module = importlib.import_module(serializer_module)
+    except ImportError, exc:
+        bad_serializer = BadSerializer(exc)
+
+        module = type('BadSerializerModule', (object,), {
+            'Deserializer': bad_serializer,
+            'Serializer': bad_serializer,
+        })
+
     if serializers is None:
         _serializers[format] = module
     else:
         serializers[format] = module
 
+
 def unregister_serializer(format):
     "Unregister a given serializer. This is not a thread-safe operation."
     if not _serializers:

tests/serializers/tests.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 from __future__ import unicode_literals
 
+import importlib
 import json
 from datetime import datetime
 import re
@@ -14,7 +15,7 @@ except ImportError:
 
 
 from django.conf import settings
-from django.core import serializers
+from django.core import management, serializers
 from django.db import transaction, connection
 from django.test import TestCase, TransactionTestCase, Approximate
 from django.utils import six
@@ -440,6 +441,69 @@ class JsonSerializerTransactionTestCase(SerializersTransactionTestBase, Transact
     }]"""
 
 
+YAML_IMPORT_ERROR_MESSAGE = r'No module named yaml'
+class YamlImportModuleMock(object):
+    """Provides a wrapped import_module function to simulate yaml ImportError
+
+    In order to run tests that verify the behavior of the YAML serializer
+    when run on a system that has yaml installed (like the django CI server),
+    mock import_module, so that it raises an ImportError when the yaml
+    serializer is being imported.  The importlib.import_module() call is
+    being made in the serializers.register_serializer().
+
+    Refs: #12756
+    """
+    def __init__(self):
+        self._import_module = importlib.import_module
+
+    def import_module(self, module_path):
+        if module_path == serializers.BUILTIN_SERIALIZERS['yaml']:
+            raise ImportError(YAML_IMPORT_ERROR_MESSAGE)
+
+        return self._import_module(module_path)
+
+
+class NoYamlSerializerTestCase(TestCase):
+    """Not having pyyaml installed provides a misleading error
+
+    Refs: #12756
+    """
+    @classmethod
+    def setUpClass(cls):
+        """Removes imported yaml and stubs importlib.import_module"""
+        super(NoYamlSerializerTestCase, cls).setUpClass()
+
+        cls._import_module_mock = YamlImportModuleMock()
+        importlib.import_module = cls._import_module_mock.import_module
+
+        # clear out cached serializers to emulate yaml missing
+        serializers._serializers = {}
+
+    @classmethod
+    def tearDownClass(cls):
+        """Puts yaml back if necessary"""
+        super(NoYamlSerializerTestCase, cls).tearDownClass()
+
+        importlib.import_module = cls._import_module_mock._import_module
+
+        # clear out cached serializers to clean out BadSerializer instances
+        serializers._serializers = {}
+
+    def test_serializer_pyyaml_error_message(self):
+        """Using yaml serializer without pyyaml raises ImportError"""
+        jane = Author(name="Jane")
+        self.assertRaises(ImportError, serializers.serialize, "yaml", [jane])
+
+    def test_deserializer_pyyaml_error_message(self):
+        """Using yaml deserializer without pyyaml raises ImportError"""
+        self.assertRaises(ImportError, serializers.deserialize, "yaml", "")
+
+    def test_dumpdata_pyyaml_error_message(self):
+        """Calling dumpdata produces an error when yaml package missing"""
+        self.assertRaisesRegexp(management.CommandError, YAML_IMPORT_ERROR_MESSAGE,
+                management.call_command, 'dumpdata', format='yaml')
+
+
 @unittest.skipUnless(HAS_YAML, "No yaml library detected")
 class YamlSerializerTestCase(SerializersTestBase, TestCase):
     serializer_name = "yaml"

tests/serializers_regress/tests.py

@@ -523,7 +523,10 @@ def streamTest(format, self):
         else:
             self.assertEqual(string_data, stream.content.decode('utf-8'))
 
-for format in serializers.get_serializer_formats():
+for format in [
+            f for f in serializers.get_serializer_formats()
+            if not isinstance(serializers.get_serializer(f), serializers.BadSerializer)
+        ]:
     setattr(SerializerTests, 'test_' + format + '_serializer', curry(serializerTest, format))
     setattr(SerializerTests, 'test_' + format + '_natural_key_serializer', curry(naturalKeySerializerTest, format))
     setattr(SerializerTests, 'test_' + format + '_serializer_fields', curry(fieldsTest, format))

tests/timezones/tests.py

@@ -599,7 +599,7 @@ class SerializationTests(TestCase):
         obj = next(serializers.deserialize('xml', data)).object
         self.assertEqual(obj.dt, dt)
 
-        if 'yaml' in serializers.get_serializer_formats():
+        if not isinstance(serializers.get_serializer('yaml'), serializers.BadSerializer):
             data = serializers.serialize('yaml', [Event(dt=dt)])
             self.assert_yaml_contains_datetime(data, "2011-09-01 13:20:30")
             obj = next(serializers.deserialize('yaml', data)).object
@@ -623,7 +623,7 @@ class SerializationTests(TestCase):
         obj = next(serializers.deserialize('xml', data)).object
         self.assertEqual(obj.dt, dt)
 
-        if 'yaml' in serializers.get_serializer_formats():
+        if not isinstance(serializers.get_serializer('yaml'), serializers.BadSerializer):
             data = serializers.serialize('yaml', [Event(dt=dt)])
             self.assert_yaml_contains_datetime(data, "2011-09-01 13:20:30.405060")
             obj = next(serializers.deserialize('yaml', data)).object
@@ -647,7 +647,7 @@ class SerializationTests(TestCase):
         obj = next(serializers.deserialize('xml', data)).object
         self.assertEqual(obj.dt, dt)
 
-        if 'yaml' in serializers.get_serializer_formats():
+        if not isinstance(serializers.get_serializer('yaml'), serializers.BadSerializer):
             data = serializers.serialize('yaml', [Event(dt=dt)])
             self.assert_yaml_contains_datetime(data, "2011-09-01 17:20:30.405060+07:00")
             obj = next(serializers.deserialize('yaml', data)).object
@@ -671,7 +671,7 @@ class SerializationTests(TestCase):
         obj = next(serializers.deserialize('xml', data)).object
         self.assertEqual(obj.dt, dt)
 
-        if 'yaml' in serializers.get_serializer_formats():
+        if not isinstance(serializers.get_serializer('yaml'), serializers.BadSerializer):
             data = serializers.serialize('yaml', [Event(dt=dt)])
             self.assert_yaml_contains_datetime(data, "2011-09-01 10:20:30+00:00")
             obj = next(serializers.deserialize('yaml', data)).object
@@ -695,7 +695,7 @@ class SerializationTests(TestCase):
         obj = next(serializers.deserialize('xml', data)).object
         self.assertEqual(obj.dt, dt)
 
-        if 'yaml' in serializers.get_serializer_formats():
+        if not isinstance(serializers.get_serializer('yaml'), serializers.BadSerializer):
             data = serializers.serialize('yaml', [Event(dt=dt)])
             self.assert_yaml_contains_datetime(data, "2011-09-01 13:20:30+03:00")
             obj = next(serializers.deserialize('yaml', data)).object
@@ -719,7 +719,7 @@ class SerializationTests(TestCase):
         obj = next(serializers.deserialize('xml', data)).object
         self.assertEqual(obj.dt, dt)
 
-        if 'yaml' in serializers.get_serializer_formats():
+        if not isinstance(serializers.get_serializer('yaml'), serializers.BadSerializer):
             data = serializers.serialize('yaml', [Event(dt=dt)])
             self.assert_yaml_contains_datetime(data, "2011-09-01 17:20:30+07:00")
             obj = next(serializers.deserialize('yaml', data)).object