[1.6.x] Merge pull request #1582 from rca/12756-missing-yaml-module-serializer-error-message

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

Backport of 4f5faa1916e7c8cb72cc9ebf1a1fd964ba6e707b from master.

6 files changed, 162 insertions, 66 deletions

diff --git a/AUTHORS b/AUTHORS
index a32d0cfe5b..2a81278658 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -57,6 +57,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>
diff --git a/django/core/management/commands/dumpdata.py b/django/core/management/commands/dumpdata.py
index c5eb1b9a9e..e2a2f24987 100644
--- a/django/core/management/commands/dumpdata.py
+++ b/django/core/management/commands/dumpdata.py
@@ -105,11 +105,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():
diff --git a/django/core/serializers/__init__.py b/django/core/serializers/__init__.py
index c48050415d..d666cd723b 100644
--- a/django/core/serializers/__init__.py
+++ b/django/core/serializers/__init__.py
@@ -26,17 +26,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.
 
@@ -52,12 +64,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:
diff --git a/tests/serializers/tests.py b/tests/serializers/tests.py
index 038276ca21..987a887f4b 100644
--- a/tests/serializers/tests.py
+++ b/tests/serializers/tests.py
@@ -1,17 +1,24 @@
+# -*- coding: utf-8 -*-
 from __future__ import absolute_import, unicode_literals
 
-# -*- coding: utf-8 -*-
 import json
 from datetime import datetime
 from xml.dom import minidom
 
+try:
+    import yaml
+    HAS_YAML = True
+except ImportError:
+    HAS_YAML = False
+
 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
 from django.utils.six import StringIO
 from django.utils import unittest
+from django.utils import importlib
 
 from .models import (Category, Author, Article, AuthorProfile, Actor, Movie,
     Score, Player, Team)
@@ -420,14 +427,74 @@ class JsonSerializerTransactionTestCase(SerializersTransactionTestBase, Transact
         }
     }]"""
 
-try:
-    import yaml
-except ImportError:
-    pass
-else:
-    class YamlSerializerTestCase(SerializersTestBase, TestCase):
-        serializer_name = "yaml"
-        fwd_ref_str = """- fields:
+
+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"
+    fwd_ref_str = """- fields:
     headline: Forward references pose no problem
     pub_date: 2006-06-16 15:00:00
     categories: [1]
@@ -443,7 +510,7 @@ else:
   pk: 1
   model: serializers.author"""
 
-        pkless_str = """- fields:
+    pkless_str = """- fields:
     name: Reference
   pk: null
   model: serializers.category
@@ -451,42 +518,44 @@ else:
     name: Non-fiction
   model: serializers.category"""
 
-        @staticmethod
-        def _validate_output(serial_str):
-            try:
-                yaml.safe_load(StringIO(serial_str))
-            except Exception:
-                return False
-            else:
-                return True
-
-        @staticmethod
-        def _get_pk_values(serial_str):
-            ret_list = []
-            stream = StringIO(serial_str)
-            for obj_dict in yaml.safe_load(stream):
-                ret_list.append(obj_dict["pk"])
-            return ret_list
-
-        @staticmethod
-        def _get_field_values(serial_str, field_name):
-            ret_list = []
-            stream = StringIO(serial_str)
-            for obj_dict in yaml.safe_load(stream):
-                if "fields" in obj_dict and field_name in obj_dict["fields"]:
-                    field_value = obj_dict["fields"][field_name]
-                    # yaml.safe_load will return non-string objects for some
-                    # of the fields we are interested in, this ensures that
-                    # everything comes back as a string
-                    if isinstance(field_value, six.string_types):
-                        ret_list.append(field_value)
-                    else:
-                        ret_list.append(str(field_value))
-            return ret_list
-
-    class YamlSerializerTransactionTestCase(SerializersTransactionTestBase, TransactionTestCase):
-        serializer_name = "yaml"
-        fwd_ref_str = """- fields:
+    @staticmethod
+    def _validate_output(serial_str):
+        try:
+            yaml.safe_load(StringIO(serial_str))
+        except Exception:
+            return False
+        else:
+            return True
+
+    @staticmethod
+    def _get_pk_values(serial_str):
+        ret_list = []
+        stream = StringIO(serial_str)
+        for obj_dict in yaml.safe_load(stream):
+            ret_list.append(obj_dict["pk"])
+        return ret_list
+
+    @staticmethod
+    def _get_field_values(serial_str, field_name):
+        ret_list = []
+        stream = StringIO(serial_str)
+        for obj_dict in yaml.safe_load(stream):
+            if "fields" in obj_dict and field_name in obj_dict["fields"]:
+                field_value = obj_dict["fields"][field_name]
+                # yaml.safe_load will return non-string objects for some
+                # of the fields we are interested in, this ensures that
+                # everything comes back as a string
+                if isinstance(field_value, six.string_types):
+                    ret_list.append(field_value)
+                else:
+                    ret_list.append(str(field_value))
+        return ret_list
+
+
+@unittest.skipUnless(HAS_YAML, "No yaml library detected")
+class YamlSerializerTransactionTestCase(SerializersTransactionTestBase, TransactionTestCase):
+    serializer_name = "yaml"
+    fwd_ref_str = """- fields:
     headline: Forward references pose no problem
     pub_date: 2006-06-16 15:00:00
     categories: [1]
diff --git a/tests/serializers_regress/tests.py b/tests/serializers_regress/tests.py
index 04b4d4c839..f5aced2baf 100644
--- a/tests/serializers_regress/tests.py
+++ b/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))
diff --git a/tests/timezones/tests.py b/tests/timezones/tests.py
index dce5c3041d..844bdb83fe 100644
--- a/tests/timezones/tests.py
+++ b/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