From 3df9647ad96eb0f1919be921bb96e949f1a518a0 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Fri, 6 Sep 2013 16:05:02 -0700 Subject: [PATCH] [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. --- AUTHORS | 1 + django/core/management/commands/dumpdata.py | 8 +- django/core/serializers/__init__.py | 39 ++++- tests/serializers/tests.py | 157 ++++++++++++++------ tests/serializers_regress/tests.py | 5 +- tests/timezones/tests.py | 12 +- 6 files changed, 159 insertions(+), 63 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 Chris Adams Mathieu Agopian + Roberto Aguilar ajs alang@bright-green.com A S Alam 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 _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_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 + @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: + +@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