Ticket #14394: validate_bad_data_assignment_to_m2m_strictly.patch

django/db/models/fields/related.py

@@ -727 +727 @@
         if not self.field.rel.through._meta.auto_created:
             opts = self.field.rel.through._meta
             raise AttributeError("Cannot set values on a ManyToManyField which specifies an intermediary model.  Use %s.%s's Manager instead." % (opts.app_label, opts.object_name))
+        
+        manager = self.__get__(instance)
 
-        manager = self.__get__(instance)
+        if not hasattr(value, '__iter__'):
+            raise TypeError(u"Cannot set non iterables on a ManyToManyField.")
+        
+        model_instances = []
+        for obj in value:
+            if isinstance(obj, manager.model):
+                if not obj.pk:
+                    raise TypeError(u"Cannot set %s to a ManyToManyField because it's not saved. Make sure you pass saved models or valid primary keys." % unicode(obj))
+                model_instances.append(obj)
+            else:
+                try:
+                    model_instances.append(manager.model.objects.get(pk=obj))
+                except (manager.model.DoesNotExist, ValueError):
+                    raise TypeError(u"Cannot set value to this ManyToManyField. Make sure you pass valid models or primary keys.")
+
         manager.clear()
-        manager.add(*value)
+        manager.add(*model_instances)
 
+
 class ManyToOneRel(object):
     def __init__(self, to, field_name, related_name=None,
             limit_choices_to=None, lookup_overrides=None, parent_link=False):

tests/regressiontests/m2m_regress/tests.py

@@ -2 +2 @@
 from django.test import TestCase
 
 from models import (SelfRefer, Tag, TagCollection, Entry, SelfReferChild,
-    SelfReferChildSibling, Worksheet)
+    SelfReferChildSibling, Worksheet, Line)
 
 
 class M2MRegressionTests(TestCase):
@@ -73 +73 @@
 
         self.assertQuerysetEqual(c1.tags.all(), ["<Tag: t1>", "<Tag: t2>"])
         self.assertQuerysetEqual(t1.tag_collections.all(), ["<TagCollection: c1>"])
+
+    def test_assigning_any_iterable_with_valid_models_to_m2m_works(self):
+        t1 = Tag.objects.create(name='t1') 
+        t2 = Tag.objects.create(name='t2') 
+        
+        c1 = TagCollection.objects.create(name='c1')
+        c1.tags = [t1,t2]
+
+        c1 = TagCollection.objects.get(pk=c1.pk)
+        self.assertQuerysetEqual(c1.tags.all(), ["<Tag: t1>", "<Tag: t2>"])
+        
+        c1.tags = set([t1, t2])
+
+        c1 = TagCollection.objects.get(pk=c1.pk)
+        self.assertQuerysetEqual(c1.tags.order_by('name'), ["<Tag: t1>", "<Tag: t2>"])
+
+        c1.tags = Tag.objects.filter(name__in=('t1', 't2'))
+
+        c1 = TagCollection.objects.get(pk=c1.pk)
+        self.assertQuerysetEqual(c1.tags.all(), ["<Tag: t1>", "<Tag: t2>"])
+        
+        c1.tags = (t1, t2)
+
+        c1 = TagCollection.objects.get(pk=c1.pk)
+        self.assertQuerysetEqual(c1.tags.all(), ["<Tag: t1>", "<Tag: t2>"])
+    
+    def test_assigning_any_iterable_with_valid_primary_keys_to_m2m_works(self):
+        t1 = Tag.objects.create(name='t1') 
+        t2 = Tag.objects.create(name='t2') 
+        
+        c1 = TagCollection.objects.create(name='c1')
+        c1.tags = [t1.pk, t2.pk]
+        
+        c1 = TagCollection.objects.get(pk=c1.pk)
+        self.assertQuerysetEqual(c1.tags.all(), ["<Tag: t1>", "<Tag: t2>"])
+
+    def test_assigning_non_iterable_to_m2m_doesnt_clear_existing_relations(self):
+        t1 = Tag.objects.create(name='t1') 
+        t2 = Tag.objects.create(name='t2') 
+        
+        c1 = TagCollection.objects.create(name='c1')
+        c1.tags = [t1,t2]
+        
+        def invalid_assigment():
+            c1.tags = 7
+
+        self.assertRaises(TypeError, invalid_assigment)
+
+        c1 = TagCollection.objects.get(pk=c1.pk)
+
+        self.assertQuerysetEqual(c1.tags.all(), ["<Tag: t1>", "<Tag: t2>"])
+    
+    def test_assigning_invalid_iterable_to_m2m_doesnt_clear_existing_relations(self):
+        t1 = Tag.objects.create(name='t1') 
+        t2 = Tag.objects.create(name='t2') 
+        
+        c1 = TagCollection.objects.create(name='c1')
+        c1.tags = [t1,t2]
+
+        def invalid_assigment():
+            c1.tags = ["foo"]
+
+        self.assertRaises(TypeError, invalid_assigment)
+
+        c1 = TagCollection.objects.get(pk=c1.pk)
+        self.assertQuerysetEqual(c1.tags.all(), ["<Tag: t1>", "<Tag: t2>"])
+
+    def test_assigning_iterable_with_invalid_pks_to_m2m_doesnt_clear_existing_relations(self):
+        t1 = Tag.objects.create(name='t1') 
+        t2 = Tag.objects.create(name='t2') 
+        
+        c1 = TagCollection.objects.create(name='c1')
+        c1.tags = [t1,t2]
+
+        def invalid_assigment():
+            c1.tags = [1234]
+
+        self.assertRaises(TypeError, invalid_assigment)
+
+        c1 = TagCollection.objects.get(pk=c1.pk)
+        self.assertQuerysetEqual(c1.tags.all(), ["<Tag: t1>", "<Tag: t2>"])
+
+    def test_assigning_iterable_with_unsaved_models_to_m2m_doesnt_clear_existing_relations(self):
+        t1 = Tag.objects.create(name='t1') 
+        t2 = Tag.objects.create(name='t2') 
+        t3 = Tag(name='t3')
+        
+        c1 = TagCollection.objects.create(name='c1')
+        c1.tags = [t1,t2]
+
+        def invalid_assigment():
+            c1.tags = [t3]
+
+        self.assertRaises(TypeError, invalid_assigment)
+
+        c1 = TagCollection.objects.get(pk=c1.pk)
+        self.assertQuerysetEqual(c1.tags.all(), ["<Tag: t1>", "<Tag: t2>"])
+
+    def test_assigning_iterable_with_wrong_models_to_m2m_doesnt_clear_existing_relations(self):
+        t1 = Tag.objects.create(name='t1') 
+        t2 = Tag.objects.create(name='t2') 
+        line = Line.objects.create(name="not a tag")
+
+        c1 = TagCollection.objects.create(name='c1')
+        c1.tags = [t1,t2]
+
+        def invalid_assigment():
+            c1.tags = [line]
+
+        self.assertRaises(TypeError, invalid_assigment)
+
+        c1 = TagCollection.objects.get(pk=c1.pk)
+        self.assertQuerysetEqual(c1.tags.all(), ["<Tag: t1>", "<Tag: t2>"])