Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #14226 -- Dependency calculation for complex M2M relations.

`sort_dependencies` incorrectly interpreted 'complex' M2M relations
(with explicit through models) as dependencies for a model. This caused
circular complex M2M relations to be unserializable by dumpdata.

Thanks to aneil for the report and outofculture for initial tests.
  • Loading branch information...
commit a75324c6544d728d3bd8f678b1b8021fdff18332 1 parent 63df886
@koirikivi koirikivi authored ramiro committed
View
14 django/core/management/commands/dumpdata.py
@@ -189,17 +189,21 @@ def sort_dependencies(app_list):
else:
deps = []
- # Now add a dependency for any FK or M2M relation with
- # a model that defines a natural key
+ # Now add a dependency for any FK relation with a model that
+ # defines a natural key
for field in model._meta.fields:
if hasattr(field.rel, 'to'):
rel_model = field.rel.to
if hasattr(rel_model, 'natural_key') and rel_model != model:
deps.append(rel_model)
+ # Also add a dependency for any simple M2M relation with a model
+ # that defines a natural key. M2M relations with explicit through
+ # models don't count as dependencies.
for field in model._meta.many_to_many:
- rel_model = field.rel.to
- if hasattr(rel_model, 'natural_key') and rel_model != model:
- deps.append(rel_model)
+ if field.rel.through._meta.auto_created:
+ rel_model = field.rel.to
+ if hasattr(rel_model, 'natural_key') and rel_model != model:
+ deps.append(rel_model)
model_dependencies.append((model, deps))
model_dependencies.reverse()
View
94 tests/fixtures_regress/models.py
@@ -235,3 +235,97 @@ def natural_key(self):
# Model for regression test of #11101
class Thingy(models.Model):
name = models.CharField(max_length=255)
+
+
+@python_2_unicode_compatible
+class BaseNKModel(models.Model):
+ """
+ Base model with a natural_key and a manager with `get_by_natural_key`
+ """
+ data = models.CharField(max_length=20, unique=True)
+ objects = NKManager()
+
+ class Meta:
+ abstract = True
+
+ def __str__(self):
+ return self.data
+
+ def natural_key(self):
+ return (self.data,)
+
+
+class M2MSimpleA(BaseNKModel):
+ b_set = models.ManyToManyField("M2MSimpleB")
+
+
+class M2MSimpleB(BaseNKModel):
+ pass
+
+
+class M2MSimpleCircularA(BaseNKModel):
+ b_set = models.ManyToManyField("M2MSimpleCircularB")
+
+
+class M2MSimpleCircularB(BaseNKModel):
+ a_set = models.ManyToManyField("M2MSimpleCircularA")
+
+
+class M2MComplexA(BaseNKModel):
+ b_set = models.ManyToManyField("M2MComplexB", through="M2MThroughAB")
+
+
+class M2MComplexB(BaseNKModel):
+ pass
+
+
+class M2MThroughAB(BaseNKModel):
+ a = models.ForeignKey(M2MComplexA)
+ b = models.ForeignKey(M2MComplexB)
+
+
+class M2MComplexCircular1A(BaseNKModel):
+ b_set = models.ManyToManyField("M2MComplexCircular1B",
+ through="M2MCircular1ThroughAB")
+
+
+class M2MComplexCircular1B(BaseNKModel):
+ c_set = models.ManyToManyField("M2MComplexCircular1C",
+ through="M2MCircular1ThroughBC")
+
+
+class M2MComplexCircular1C(BaseNKModel):
+ a_set = models.ManyToManyField("M2MComplexCircular1A",
+ through="M2MCircular1ThroughCA")
+
+
+class M2MCircular1ThroughAB(BaseNKModel):
+ a = models.ForeignKey(M2MComplexCircular1A)
+ b = models.ForeignKey(M2MComplexCircular1B)
+
+
+class M2MCircular1ThroughBC(BaseNKModel):
+ b = models.ForeignKey(M2MComplexCircular1B)
+ c = models.ForeignKey(M2MComplexCircular1C)
+
+
+class M2MCircular1ThroughCA(BaseNKModel):
+ c = models.ForeignKey(M2MComplexCircular1C)
+ a = models.ForeignKey(M2MComplexCircular1A)
+
+
+class M2MComplexCircular2A(BaseNKModel):
+ b_set = models.ManyToManyField("M2MComplexCircular2B",
+ through="M2MCircular2ThroughAB")
+
+
+class M2MComplexCircular2B(BaseNKModel):
+ def natural_key(self):
+ return (self.data,)
+ # Fake the dependency for a circularity
+ natural_key.dependencies = ["fixtures_regress.M2MComplexCircular2A"]
+
+
+class M2MCircular2ThroughAB(BaseNKModel):
+ a = models.ForeignKey(M2MComplexCircular2A)
+ b = models.ForeignKey(M2MComplexCircular2B)
View
125 tests/fixtures_regress/tests.py
@@ -7,6 +7,7 @@
import re
import warnings
+from django.core import serializers
from django.core.serializers.base import DeserializationError
from django.core import management
from django.core.management.base import CommandError
@@ -23,7 +24,12 @@
from .models import (Animal, Stuff, Absolute, Parent, Child, Article, Widget,
Store, Person, Book, NKChild, RefToNKChild, Circle1, Circle2, Circle3,
- ExternalDependency, Thingy)
+ ExternalDependency, Thingy,
+ M2MSimpleA, M2MSimpleB, M2MSimpleCircularA, M2MSimpleCircularB,
+ M2MComplexA, M2MComplexB, M2MThroughAB, M2MComplexCircular1A,
+ M2MComplexCircular1B, M2MComplexCircular1C, M2MCircular1ThroughAB,
+ M2MCircular1ThroughBC, M2MCircular1ThroughCA, M2MComplexCircular2A,
+ M2MComplexCircular2B, M2MCircular2ThroughAB)
_cur_dir = os.path.dirname(os.path.abspath(upath(__file__)))
@@ -702,6 +708,123 @@ def test_normal_pk(self):
)
+class M2MNaturalKeyFixtureTests(TestCase):
+ """Tests for ticket #14426."""
+
+ def test_dependency_sorting_m2m_simple(self):
+ """
+ M2M relations without explicit through models SHOULD count as dependencies
+
+ Regression test for bugs that could be caused by flawed fixes to
+ #14226, namely if M2M checks are removed from sort_dependencies
+ altogether.
+ """
+ sorted_deps = sort_dependencies(
+ [('fixtures_regress', [M2MSimpleA, M2MSimpleB])]
+ )
+ self.assertEqual(sorted_deps, [M2MSimpleB, M2MSimpleA])
+
+ def test_dependency_sorting_m2m_simple_circular(self):
+ """
+ Resolving circular M2M relations without explicit through models should
+ fail loudly
+ """
+ self.assertRaisesMessage(
+ CommandError,
+ "Can't resolve dependencies for fixtures_regress.M2MSimpleCircularA, "
+ "fixtures_regress.M2MSimpleCircularB in serialized app list.",
+ sort_dependencies,
+ [('fixtures_regress', [M2MSimpleCircularA, M2MSimpleCircularB])]
+ )
+
+ def test_dependency_sorting_m2m_complex(self):
+ """
+ M2M relations with explicit through models should NOT count as
+ dependencies. The through model itself will have dependencies, though.
+ """
+ sorted_deps = sort_dependencies(
+ [('fixtures_regress', [M2MComplexA, M2MComplexB, M2MThroughAB])]
+ )
+ # Order between M2MComplexA and M2MComplexB doesn't matter. The through
+ # model has dependencies to them though, so it should come last.
+ self.assertEqual(sorted_deps[-1], M2MThroughAB)
+
+ def test_dependency_sorting_m2m_complex_circular_1(self):
+ """
+ Circular M2M relations with explicit through models should be serializable
+ """
+ A, B, C, AtoB, BtoC, CtoA = (M2MComplexCircular1A, M2MComplexCircular1B,
+ M2MComplexCircular1C, M2MCircular1ThroughAB,
+ M2MCircular1ThroughBC, M2MCircular1ThroughCA)
+ try:
+ sorted_deps = sort_dependencies(
+ [('fixtures_regress', [A, B, C, AtoB, BtoC, CtoA])]
+ )
+ except CommandError:
+ self.fail("Serialization dependency solving algorithm isn't "
+ "capable of handling circular M2M setups with "
+ "intermediate models.")
+
+ # The dependency sorting should not result in an error, and the
+ # through model should have dependencies to the other models and as
+ # such come last in the list.
+ self.assertEqual(sorted(sorted_deps[:3]), sorted([A, B, C]))
+ self.assertEqual(sorted(sorted_deps[3:]), sorted([AtoB, BtoC, CtoA]))
+
+ def test_dependency_sorting_m2m_complex_circular_2(self):
+ """
+ Circular M2M relations with explicit through models should be serializable
+ This test tests the circularity with explicit natural_key.dependencies
+ """
+ try:
+ sorted_deps = sort_dependencies(
+ [('fixtures_regress', [
+ M2MComplexCircular2A,
+ M2MComplexCircular2B,
+ M2MCircular2ThroughAB]
+ )
+ ]
+ )
+ except CommandError:
+ self.fail("Serialization dependency solving algorithm isn't "
+ "capable of handling circular M2M setups with "
+ "intermediate models plus natural key dependency hints.")
+ self.assertEqual(sorted(sorted_deps[:2]), sorted([M2MComplexCircular2A, M2MComplexCircular2B]))
+ self.assertEqual(sorted_deps[2:], [M2MCircular2ThroughAB])
+
+ def test_dump_and_load_m2m_simple(self):
+ """
+ Test serializing and deserializing back models with simple M2M relations
+ """
+ a = M2MSimpleA.objects.create(data="a")
+ b1 = M2MSimpleB.objects.create(data="b1")
+ b2 = M2MSimpleB.objects.create(data="b2")
+ a.b_set.add(b1)
+ a.b_set.add(b2)
+
+ stdout = StringIO()
+ management.call_command(
+ 'dumpdata',
+ 'fixtures_regress.M2MSimpleA',
+ 'fixtures_regress.M2MSimpleB',
+ use_natural_foreign_keys=True,
+ stdout=stdout
+ )
+
+ for model in [M2MSimpleA, M2MSimpleB]:
+ model.objects.all().delete()
+
+ objects = serializers.deserialize("json", stdout.getvalue())
+ for obj in objects:
+ obj.save()
+
+ new_a = M2MSimpleA.objects.get_by_natural_key("a")
+ self.assertQuerysetEqual(new_a.b_set.all(), [
+ "<M2MSimpleB: b1>",
+ "<M2MSimpleB: b2>"
+ ], ordered=False)
+
+
class TestTicket11101(TransactionTestCase):
available_apps = [
Please sign in to comment.
Something went wrong with that request. Please try again.