Browse Source

Fixed #26521 -- Validated CreateModel bases, fields and managers for duplicates.

James Robert 9 years ago
parent
commit
417e083e55

+ 20 - 0
django/db/migrations/operations/models.py

@@ -12,6 +12,16 @@ from .fields import (
 )
 
 
+def _check_for_duplicates(arg_name, objs):
+    used_vals = set()
+    for val in objs:
+        if val in used_vals:
+            raise ValueError(
+                'Found duplicate %s in CreateModel operation.' % arg_name
+            )
+        used_vals.add(val)
+
+
 class ModelOperation(Operation):
     def __init__(self, name):
         self.name = name
@@ -43,6 +53,16 @@ class CreateModel(ModelOperation):
         self.bases = bases or (models.Model,)
         self.managers = managers or []
         super(CreateModel, self).__init__(name)
+        # Sanity-check that there are no duplicated field names, bases, or
+        # manager names
+        _check_for_duplicates("field", (name for name, _ in self.fields))
+        _check_for_duplicates(
+            "base",
+            (base._meta.label_lower if isinstance(base, models.base.ModelBase) else base.lower()
+             for base in self.bases
+             if base is not models.Model)
+        )
+        _check_for_duplicates("manager", (name for name, _ in self.managers))
 
     def deconstruct(self):
         kwargs = {

+ 55 - 1
tests/migrations/test_operations.py

@@ -10,7 +10,7 @@ from django.db.transaction import atomic
 from django.db.utils import IntegrityError
 from django.test import override_settings, skipUnlessDBFeature
 
-from .models import FoodManager, FoodQuerySet
+from .models import FoodManager, FoodQuerySet, UnicodeModel
 from .test_base import MigrationTestBase
 
 try:
@@ -200,6 +200,60 @@ class OperationTests(OperationTestBase):
         definition = operation.deconstruct()
         self.assertNotIn('managers', definition[2])
 
+    def test_create_model_with_duplicate_field_name(self):
+        with self.assertRaisesMessage(ValueError, "Found duplicate field in CreateModel operation."):
+            migrations.CreateModel(
+                "Pony",
+                [
+                    ("id", models.AutoField(primary_key=True)),
+                    ("pink", models.TextField()),
+                    ("pink", models.IntegerField(default=1)),
+                ],
+            )
+
+    def test_create_model_with_duplicate_base(self):
+        with self.assertRaisesMessage(ValueError, "Found duplicate base in CreateModel operation."):
+            migrations.CreateModel(
+                "Pony",
+                fields=[],
+                bases=("test_crmo.Pony", "test_crmo.Pony",),
+            )
+        with self.assertRaisesMessage(ValueError, "Found duplicate base in CreateModel operation."):
+            migrations.CreateModel(
+                "Pony",
+                fields=[],
+                bases=(UnicodeModel, UnicodeModel,),
+            )
+        with self.assertRaisesMessage(ValueError, "Found duplicate base in CreateModel operation."):
+            migrations.CreateModel(
+                "Pony",
+                fields=[],
+                bases=("test_crmo.Pony", "test_crmo.pony",),
+            )
+        with self.assertRaisesMessage(ValueError, "Found duplicate base in CreateModel operation."):
+            migrations.CreateModel(
+                "Pony",
+                fields=[],
+                bases=(UnicodeModel, 'migrations.unicodemodel',),
+            )
+        with self.assertRaisesMessage(ValueError, "Found duplicate base in CreateModel operation."):
+            migrations.CreateModel(
+                "Pony",
+                fields=[],
+                bases=(UnicodeModel, 'migrations.UnicodeModel',),
+            )
+
+    def test_create_model_with_duplicate_manager_name(self):
+        with self.assertRaisesMessage(ValueError, "Found duplicate manager in CreateModel operation."):
+            migrations.CreateModel(
+                "Pony",
+                fields=[],
+                managers=[
+                    ("objects", models.Manager()),
+                    ("objects", models.Manager()),
+                ],
+            )
+
     def test_create_model_with_unique_after(self):
         """
         Tests the CreateModel operation directly followed by an

+ 11 - 11
tests/migrations/test_optimizer.py

@@ -5,7 +5,7 @@ from django.db.migrations import operations
 from django.db.migrations.optimizer import MigrationOptimizer
 from django.test import SimpleTestCase
 
-from .models import CustomModelBase, EmptyManager
+from .models import EmptyManager, UnicodeModel
 
 
 class OptimizerTests(SimpleTestCase):
@@ -71,7 +71,7 @@ class OptimizerTests(SimpleTestCase):
                     name="Foo",
                     fields=[("name", models.CharField(max_length=255))],
                     options={'verbose_name': 'Foo'},
-                    bases=(CustomModelBase),
+                    bases=(UnicodeModel,),
                     managers=managers,
                 ),
                 migrations.RenameModel("Foo", "Bar"),
@@ -81,7 +81,7 @@ class OptimizerTests(SimpleTestCase):
                     "Bar",
                     [("name", models.CharField(max_length=255))],
                     options={'verbose_name': 'Foo'},
-                    bases=(CustomModelBase),
+                    bases=(UnicodeModel,),
                     managers=managers,
                 )
             ],
@@ -237,7 +237,7 @@ class OptimizerTests(SimpleTestCase):
                     name="Foo",
                     fields=[("name", models.CharField(max_length=255))],
                     options={'verbose_name': 'Foo'},
-                    bases=(CustomModelBase),
+                    bases=(UnicodeModel,),
                     managers=managers,
                 ),
                 migrations.AddField("Foo", "age", models.IntegerField()),
@@ -250,7 +250,7 @@ class OptimizerTests(SimpleTestCase):
                         ("age", models.IntegerField()),
                     ],
                     options={'verbose_name': 'Foo'},
-                    bases=(CustomModelBase),
+                    bases=(UnicodeModel,),
                     managers=managers,
                 ),
             ],
@@ -309,7 +309,7 @@ class OptimizerTests(SimpleTestCase):
                     name="Foo",
                     fields=[("name", models.CharField(max_length=255))],
                     options={'verbose_name': 'Foo'},
-                    bases=(CustomModelBase),
+                    bases=(UnicodeModel,),
                     managers=managers,
                 ),
                 migrations.AlterField("Foo", "name", models.IntegerField()),
@@ -321,7 +321,7 @@ class OptimizerTests(SimpleTestCase):
                         ("name", models.IntegerField()),
                     ],
                     options={'verbose_name': 'Foo'},
-                    bases=(CustomModelBase),
+                    bases=(UnicodeModel,),
                     managers=managers,
                 ),
             ],
@@ -338,7 +338,7 @@ class OptimizerTests(SimpleTestCase):
                     name="Foo",
                     fields=[("name", models.CharField(max_length=255))],
                     options={'verbose_name': 'Foo'},
-                    bases=(CustomModelBase),
+                    bases=(UnicodeModel,),
                     managers=managers,
                 ),
                 migrations.RenameField("Foo", "name", "title"),
@@ -350,7 +350,7 @@ class OptimizerTests(SimpleTestCase):
                         ("title", models.CharField(max_length=255)),
                     ],
                     options={'verbose_name': 'Foo'},
-                    bases=(CustomModelBase),
+                    bases=(UnicodeModel,),
                     managers=managers,
                 ),
             ],
@@ -401,7 +401,7 @@ class OptimizerTests(SimpleTestCase):
                         ("age", models.IntegerField()),
                     ],
                     options={'verbose_name': 'Foo'},
-                    bases=(CustomModelBase),
+                    bases=(UnicodeModel,),
                     managers=managers,
                 ),
                 migrations.RemoveField("Foo", "age"),
@@ -413,7 +413,7 @@ class OptimizerTests(SimpleTestCase):
                         ("name", models.CharField(max_length=255)),
                     ],
                     options={'verbose_name': 'Foo'},
-                    bases=(CustomModelBase),
+                    bases=(UnicodeModel,),
                     managers=managers,
                 ),
             ],