Fixed #19820 -- Added more helpful error messages to Python deserializer.

4 files changed, 228 insertions, 17 deletions

diff --git a/django/core/serializers/base.py b/django/core/serializers/base.py
index 2c18d7b477..0f7535a148 100644
--- a/django/core/serializers/base.py
+++ b/django/core/serializers/base.py
@@ -17,7 +17,14 @@ class SerializationError(Exception):
 
 class DeserializationError(Exception):
     """Something bad happened during deserialization."""
-    pass
+
+    @classmethod
+    def WithData(cls, original_exc, model, fk, field_value):
+        """
+        Factory method for creating a deserialization error which has a more
+        explanatory messsage.
+        """
+        return cls("%s: (%s:pk=%s) field_value was '%s'" % (original_exc, model, fk, field_value))
 
 
 class Serializer(object):
diff --git a/django/core/serializers/python.py b/django/core/serializers/python.py
index d209f7ebd0..c97ab627a3 100644
--- a/django/core/serializers/python.py
+++ b/django/core/serializers/python.py
@@ -100,7 +100,10 @@ def Deserializer(object_list, **options):
                 raise
         data = {}
         if 'pk' in d:
-            data[Model._meta.pk.attname] = Model._meta.pk.to_python(d.get("pk", None))
+            try:
+                data[Model._meta.pk.attname] = Model._meta.pk.to_python(d.get('pk'))
+            except Exception as e:
+                raise base.DeserializationError.WithData(e, d['model'], d.get('pk'), None)
         m2m_data = {}
         field_names = {f.name for f in Model._meta.get_fields()}
 
@@ -128,30 +131,42 @@ def Deserializer(object_list, **options):
                             return force_text(field.remote_field.model._meta.pk.to_python(value), strings_only=True)
                 else:
                     m2m_convert = lambda v: force_text(field.remote_field.model._meta.pk.to_python(v), strings_only=True)
-                m2m_data[field.name] = [m2m_convert(pk) for pk in field_value]
+
+                try:
+                    m2m_data[field.name] = []
+                    for pk in field_value:
+                        m2m_data[field.name].append(m2m_convert(pk))
+                except Exception as e:
+                    raise base.DeserializationError.WithData(e, d['model'], d.get('pk'), pk)
 
             # Handle FK fields
             elif field.remote_field and isinstance(field.remote_field, models.ManyToOneRel):
                 if field_value is not None:
-                    if hasattr(field.remote_field.model._default_manager, 'get_by_natural_key'):
-                        if hasattr(field_value, '__iter__') and not isinstance(field_value, six.text_type):
-                            obj = field.remote_field.model._default_manager.db_manager(db).get_by_natural_key(*field_value)
-                            value = getattr(obj, field.remote_field.field_name)
-                            # If this is a natural foreign key to an object that
-                            # has a FK/O2O as the foreign key, use the FK value
-                            if field.remote_field.model._meta.pk.remote_field:
-                                value = value.pk
+                    try:
+                        if hasattr(field.remote_field.model._default_manager, 'get_by_natural_key'):
+                            if hasattr(field_value, '__iter__') and not isinstance(field_value, six.text_type):
+                                obj = field.remote_field.model._default_manager.db_manager(db).get_by_natural_key(*field_value)
+                                value = getattr(obj, field.remote_field.field_name)
+                                # If this is a natural foreign key to an object that
+                                # has a FK/O2O as the foreign key, use the FK value
+                                if field.remote_field.model._meta.pk.remote_field:
+                                    value = value.pk
+                            else:
+                                value = field.remote_field.model._meta.get_field(field.remote_field.field_name).to_python(field_value)
+                            data[field.attname] = value
                         else:
-                            value = field.remote_field.model._meta.get_field(field.remote_field.field_name).to_python(field_value)
-                        data[field.attname] = value
-                    else:
-                        data[field.attname] = field.remote_field.model._meta.get_field(field.remote_field.field_name).to_python(field_value)
+                            data[field.attname] = field.remote_field.model._meta.get_field(field.remote_field.field_name).to_python(field_value)
+                    except Exception as e:
+                        raise base.DeserializationError.WithData(e, d['model'], d.get('pk'), field_value)
                 else:
                     data[field.attname] = None
 
             # Handle all other fields
             else:
-                data[field.name] = field.to_python(field_value)
+                try:
+                    data[field.name] = field.to_python(field_value)
+                except Exception as e:
+                    raise base.DeserializationError.WithData(e, d['model'], d.get('pk'), field_value)
 
         obj = base.build_instance(Model, data, db)
         yield base.DeserializedObject(obj, m2m_data)
diff --git a/tests/serializers/models.py b/tests/serializers/models.py
index 08dc860821..5e7a31902e 100644
--- a/tests/serializers/models.py
+++ b/tests/serializers/models.py
@@ -14,9 +14,33 @@ from django.utils import six
 from django.utils.encoding import python_2_unicode_compatible
 
 
+class CategoryMetaDataManager(models.Manager):
+
+    def get_by_natural_key(self, kind, name):
+        return self.get(kind=kind, name=name)
+
+
+@python_2_unicode_compatible
+class CategoryMetaData(models.Model):
+    kind = models.CharField(max_length=10)
+    name = models.CharField(max_length=10)
+    value = models.CharField(max_length=10)
+    objects = CategoryMetaDataManager()
+
+    class Meta:
+        unique_together = (('kind', 'name'),)
+
+    def __str__(self):
+        return '[%s:%s]=%s' % (self.kind, self.name, self.value)
+
+    def natural_key(self):
+        return (self.kind, self.name)
+
+
 @python_2_unicode_compatible
 class Category(models.Model):
     name = models.CharField(max_length=20)
+    meta_data = models.ForeignKey(CategoryMetaData, null=True, default=None)
 
     class Meta:
         ordering = ('name',)
@@ -42,6 +66,7 @@ class Article(models.Model):
     headline = models.CharField(max_length=50)
     pub_date = models.DateTimeField()
     categories = models.ManyToManyField(Category)
+    meta_data = models.ManyToManyField(CategoryMetaData)
 
     class Meta:
         ordering = ('pub_date',)
diff --git a/tests/serializers/tests.py b/tests/serializers/tests.py
index 1d0a0bead5..db2f07d41d 100644
--- a/tests/serializers/tests.py
+++ b/tests/serializers/tests.py
@@ -321,6 +321,7 @@ class XmlSerializerTestCase(SerializersTestBase, TestCase):
     <field name="headline" type="CharField">Poker has no place on ESPN</field>
     <field name="pub_date" type="DateTimeField">2006-06-16T11:00:00</field>
     <field name="categories" rel="ManyToManyRel" to="serializers.category"><object pk="%(first_category_pk)s"></object><object pk="%(second_category_pk)s"></object></field>
+    <field name="meta_data" rel="ManyToManyRel" to="serializers.categorymetadata"></field>
   </object>
 </django-objects>"""
 
@@ -373,6 +374,7 @@ class XmlSerializerTransactionTestCase(SerializersTransactionTestBase, Transacti
         <field to="serializers.category" name="categories" rel="ManyToManyRel">
             <object pk="1"></object>
         </field>
+        <field to="serializers.categorymetadata" name="meta_data" rel="ManyToManyRel"></field>
     </object>
     <object pk="1" model="serializers.author">
         <field type="CharField" name="name">Agnes</field>
@@ -404,7 +406,8 @@ class JsonSerializerTestCase(SerializersTestBase, TestCase):
     "categories": [
       %(first_category_pk)s,
       %(second_category_pk)s
-    ]
+    ],
+    "meta_data": []
   }
 }
 ]
@@ -447,6 +450,166 @@ class JsonSerializerTestCase(SerializersTestBase, TestCase):
             if re.search(r'.+,\s*$', line):
                 self.assertEqual(line, line.rstrip())
 
+    def test_helpful_error_message_invalid_pk(self):
+        """
+        If there is an invalid primary key, the error message should contain
+        the model associated with it.
+        """
+        test_string = """[{
+            "pk": "badpk",
+            "model": "serializers.player",
+            "fields": {
+                "name": "Bob",
+                "rank": 1,
+                "team": "Team"
+            }
+        }]"""
+        with self.assertRaisesMessage(serializers.base.DeserializationError, "(serializers.player:pk=badpk)"):
+            list(serializers.deserialize('json', test_string))
+
+    def test_helpful_error_message_invalid_field(self):
+        """
+        If there is an invalid field value, the error message should contain
+        the model associated with it.
+        """
+        test_string = """[{
+            "pk": "1",
+            "model": "serializers.player",
+            "fields": {
+                "name": "Bob",
+                "rank": "invalidint",
+                "team": "Team"
+            }
+        }]"""
+        expected = "(serializers.player:pk=1) field_value was 'invalidint'"
+        with self.assertRaisesMessage(serializers.base.DeserializationError, expected):
+            list(serializers.deserialize('json', test_string))
+
+    def test_helpful_error_message_for_foreign_keys(self):
+        """
+        Invalid foreign keys with a natural key should throw a helpful error
+        message, such as what the failing key is.
+        """
+        test_string = """[{
+            "pk": 1,
+            "model": "serializers.category",
+            "fields": {
+                "name": "Unknown foreign key",
+                "meta_data": [
+                    "doesnotexist",
+                    "metadata"
+                ]
+            }
+        }]"""
+        key = ["doesnotexist", "metadata"]
+        expected = "(serializers.category:pk=1) field_value was '%r'" % key
+        with self.assertRaisesMessage(serializers.base.DeserializationError, expected):
+            list(serializers.deserialize('json', test_string))
+
+    def test_helpful_error_message_for_many2many_non_natural(self):
+        """
+        Invalid many-to-many keys should throw a helpful error message.
+        """
+        test_string = """[{
+            "pk": 1,
+            "model": "serializers.article",
+            "fields": {
+                "author": 1,
+                "headline": "Unknown many to many",
+                "pub_date": "2014-09-15T10:35:00",
+                "categories": [1, "doesnotexist"]
+            }
+        }, {
+            "pk": 1,
+            "model": "serializers.author",
+            "fields": {
+                "name": "Agnes"
+            }
+        }, {
+            "pk": 1,
+            "model": "serializers.category",
+            "fields": {
+                "name": "Reference"
+            }
+        }]"""
+        expected = "(serializers.article:pk=1) field_value was 'doesnotexist'"
+        with self.assertRaisesMessage(serializers.base.DeserializationError, expected):
+            list(serializers.deserialize('json', test_string))
+
+    def test_helpful_error_message_for_many2many_natural1(self):
+        """
+        Invalid many-to-many keys should throw a helpful error message.
+        This tests the code path where one of a list of natural keys is invalid.
+        """
+        test_string = """[{
+            "pk": 1,
+            "model": "serializers.categorymetadata",
+            "fields": {
+                "kind": "author",
+                "name": "meta1",
+                "value": "Agnes"
+            }
+        }, {
+            "pk": 1,
+            "model": "serializers.article",
+            "fields": {
+                "author": 1,
+                "headline": "Unknown many to many",
+                "pub_date": "2014-09-15T10:35:00",
+                "meta_data": [
+                    ["author", "meta1"],
+                    ["doesnotexist", "meta1"],
+                    ["author", "meta1"]
+                ]
+            }
+        }, {
+            "pk": 1,
+            "model": "serializers.author",
+            "fields": {
+                "name": "Agnes"
+            }
+        }]"""
+        key = ["doesnotexist", "meta1"]
+        expected = "(serializers.article:pk=1) field_value was '%r'" % key
+        with self.assertRaisesMessage(serializers.base.DeserializationError, expected):
+            for obj in serializers.deserialize('json', test_string):
+                obj.save()
+
+    def test_helpful_error_message_for_many2many_natural2(self):
+        """
+        Invalid many-to-many keys should throw a helpful error message. This
+        tests the code path where a natural many-to-many key has only a single
+        value.
+        """
+        test_string = """[{
+            "pk": 1,
+            "model": "serializers.article",
+            "fields": {
+                "author": 1,
+                "headline": "Unknown many to many",
+                "pub_date": "2014-09-15T10:35:00",
+                "meta_data": [1, "doesnotexist"]
+            }
+        }, {
+            "pk": 1,
+            "model": "serializers.categorymetadata",
+            "fields": {
+                "kind": "author",
+                "name": "meta1",
+                "value": "Agnes"
+            }
+        }, {
+            "pk": 1,
+            "model": "serializers.author",
+            "fields": {
+                "name": "Agnes"
+            }
+        }]"""
+        expected = "(serializers.article:pk=1) field_value was 'doesnotexist'"
+        with self.assertRaisesMessage(serializers.base.DeserializationError, expected):
+            for obj in serializers.deserialize('json', test_string, ignore=False):
+                obj.save()
+
 
 class JsonSerializerTransactionTestCase(SerializersTransactionTestBase, TransactionTestCase):
     serializer_name = "json"
@@ -576,6 +739,7 @@ class YamlSerializerTestCase(SerializersTestBase, TestCase):
     headline: Poker has no place on ESPN
     pub_date: 2006-06-16 11:00:00
     categories: [%(first_category_pk)s, %(second_category_pk)s]
+    meta_data: []
 """
 
     @staticmethod