Ticket #17673: 17673.diff

django/core/management/validation.py

@@ -32 +32 @@
 
     for cls in models.get_models(app):
         opts = cls._meta
+        # Keep record of used field names. Shadowing field names in inheritance
+        # cases is not allowed as this will lead to buggy behavior. See ticket
+        # #17673 for details.
+        # It is also possible to define clashing names inside a single model:
+        #   f = models.ForeignKey()
+        #   f_id = models.IntegerField
+        # => f.attname clashes with f_id.name.
+        used_fields = {} # name or attname -> field
+        for parent in opts.parents:
+            for f in parent._meta.local_fields:
+                # Check that multi-inheritance doesn't cause field name
+                # shadowing.
+                clash = used_fields.get(f.name) or used_fields.get(f.attname) or None
+                if clash:
+                    e.add(opts, 'The field "%s" from parent model "%s" clashes with the field '
+                                '"%s" from another parent model "%s"' % (
+                          f.name, f.model._meta, clash.name, clash.model._meta
+                    ))
+                used_fields[f.name] = f
+                used_fields[f.attname] = f
 
         # Do field-specific validation.
         for f in opts.local_fields:
+            clash = used_fields.get(f.name) or used_fields.get(f.attname) or None
+            if clash:
+                e.add(opts, '"%s": This field clashes with field "%s" from "%s"' %
+                            (f.name, clash.name, clash.model._meta))
+            used_fields[f.name] = f
+            used_fields[f.attname] = f
+            if f.name == 'pk':
+                e.add(opts, '"%s": You can\'t use "pk" as a field name. It is a reserved name.' %
+                                   f.name)
+            if '__' in f.name:
+                e.add(opts, '"%s": Field\'s name must not contain "__".' % f.name)
             if f.name == 'id' and not f.primary_key and opts.pk.name == 'id':
                 e.add(opts, '"%s": You can\'t use "id" as a field name, because each model automatically gets an "id" field if none of the fields have primary_key=True. You need to either remove/rename your "id" field or add primary_key=True to a field.' % f.name)
             if f.name.endswith('_'):

tests/modeltests/invalid_models/invalid_models/models.py

@@ -243 +243 @@
     class Meta:
         ordering = ('pk',)
 
+class InvalidFieldNames(models.Model):
+    pk = models.IntegerField()
+    some__field = models.IntegerField()
+
+class Parent1(models.Model):
+    somef_id = models.IntegerField()
+    someotherf = models.IntegerField()
+
+class Parent2(models.Model):
+    somef_id = models.IntegerField()
+
+class Child1(Parent1):
+    somef = models.ForeignKey(Parent2)
+
+class MultiInheritanceClash(Parent1, Parent2):
+    # Here we have two clashed: id (automatic field) and somef, because
+    # both parents define these fields.
+    pass
+
+class InternalClashingNames(models.Model):
+    # fk.attname must not clash with fk_id.name
+    fk = models.ForeignKey(Parent1)
+    fk_id = models.IntegerField()
+
 model_errors = """invalid_models.fielderrors: "charfield": CharFields require a "max_length" attribute that is a positive integer.
 invalid_models.fielderrors: "charfield2": CharFields require a "max_length" attribute that is a positive integer.
 invalid_models.fielderrors: "charfield3": CharFields require a "max_length" attribute that is a positive integer.
@@ -351 +375 @@
 invalid_models.nonexistingorderingwithsingleunderscore: "ordering" refers to "does_not_exist", a field that doesn't exist.
 invalid_models.invalidsetnull: 'fk' specifies on_delete=SET_NULL, but cannot be null.
 invalid_models.invalidsetdefault: 'fk' specifies on_delete=SET_DEFAULT, but has no default value.
+invalid_models.invalidfieldnames: "pk": You can't use "pk" as a field name. It is a reserved name.
+invalid_models.invalidfieldnames: "some__field": Field's name must not contain "__".
+invalid_models.child1: "somef": This field clashes with field "somef_id" from "invalid_models.parent1"
+invalid_models.multiinheritanceclash: The field "id" from parent model "invalid_models.parent2" clashes with the field "id" from another parent model "invalid_models.parent1"
+invalid_models.multiinheritanceclash: The field "somef_id" from parent model "invalid_models.parent2" clashes with the field "somef_id" from another parent model "invalid_models.parent1"
+invalid_models.internalclashingnames: "fk_id": This field clashes with field "fk" from "invalid_models.internalclashingnames"
 """
 
 if not connection.features.interprets_empty_strings_as_nulls:

tests/modeltests/model_inheritance/models.py

@@ -38 +38 @@
     class Meta:
         pass
 
-class StudentWorker(Student, Worker):
-    pass
+# This is invalid. The parent student and worker can have different names,
+# ages and most of all different id. However, the child model can have just
+# single name, age and id. So, what you get in Python doesn't match what you
+# have in the DB.
+#class StudentWorker(Student, Worker):
+#    pass
 
 #
 # Abstract base classes with related models

tests/modeltests/model_inheritance/tests.py

@@ -6 +6 @@
 from django.test import TestCase
 
 from .models import (Chef, CommonInfo, ItalianRestaurant, ParkingLot, Place,
-    Post, Restaurant, Student, StudentWorker, Supplier, Worker, MixinModel)
+    Post, Restaurant, Student, Supplier, Worker, MixinModel)
 
 
 class ModelInheritanceTests(TestCase):
@@ -45 +45 @@
 
         # A StudentWorker which does not exist is both a Student and Worker
         # which does not exist.
-        self.assertRaises(Student.DoesNotExist,
-            StudentWorker.objects.get, pk=12321321
-        )
-        self.assertRaises(Worker.DoesNotExist,
-            StudentWorker.objects.get, pk=12321321
-        )
+        #self.assertRaises(Student.DoesNotExist,
+        #    StudentWorker.objects.get, pk=12321321
+        #)
+        #self.assertRaises(Worker.DoesNotExist,
+        #    StudentWorker.objects.get, pk=12321321
+        #)
 
         # MultipleObjectsReturned is also inherited.
         # This is written out "long form", rather than using __init__/create()
         # because of a bug with diamond inheritance (#10808)
-        sw1 = StudentWorker()
-        sw1.name = "Wilma"
-        sw1.age = 35
-        sw1.save()
-        sw2 = StudentWorker()
-        sw2.name = "Betty"
-        sw2.age = 24
-        sw2.save()
-
-        self.assertRaises(Student.MultipleObjectsReturned,
-            StudentWorker.objects.get, pk__lt=sw2.pk + 100
-        )
-        self.assertRaises(Worker.MultipleObjectsReturned,
-            StudentWorker.objects.get, pk__lt=sw2.pk + 100
-        )
+        #sw1 = StudentWorker()
+        #sw1.name = "Wilma"
+        #sw1.age = 35
+        #sw1.save()
+        #sw2 = StudentWorker()
+        #sw2.name = "Betty"
+        #sw2.age = 24
+        #sw2.save()
+
+        #self.assertRaises(Student.MultipleObjectsReturned,
+        #    StudentWorker.objects.get, pk__lt=sw2.pk + 100
+        #)
+        #self.assertRaises(Worker.MultipleObjectsReturned,
+        #    StudentWorker.objects.get, pk__lt=sw2.pk + 100
+        #)
 
     def test_multiple_table(self):
         post = Post.objects.create(title="Lorem Ipsum")