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.
This commit is contained in:
parent
63df886df7
commit
a75324c654
|
@ -189,17 +189,21 @@ def sort_dependencies(app_list):
|
||||||
else:
|
else:
|
||||||
deps = []
|
deps = []
|
||||||
|
|
||||||
# Now add a dependency for any FK or M2M relation with
|
# Now add a dependency for any FK relation with a model that
|
||||||
# a model that defines a natural key
|
# defines a natural key
|
||||||
for field in model._meta.fields:
|
for field in model._meta.fields:
|
||||||
if hasattr(field.rel, 'to'):
|
if hasattr(field.rel, 'to'):
|
||||||
rel_model = field.rel.to
|
rel_model = field.rel.to
|
||||||
if hasattr(rel_model, 'natural_key') and rel_model != model:
|
if hasattr(rel_model, 'natural_key') and rel_model != model:
|
||||||
deps.append(rel_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:
|
for field in model._meta.many_to_many:
|
||||||
rel_model = field.rel.to
|
if field.rel.through._meta.auto_created:
|
||||||
if hasattr(rel_model, 'natural_key') and rel_model != model:
|
rel_model = field.rel.to
|
||||||
deps.append(rel_model)
|
if hasattr(rel_model, 'natural_key') and rel_model != model:
|
||||||
|
deps.append(rel_model)
|
||||||
model_dependencies.append((model, deps))
|
model_dependencies.append((model, deps))
|
||||||
|
|
||||||
model_dependencies.reverse()
|
model_dependencies.reverse()
|
||||||
|
|
|
@ -235,3 +235,97 @@ class ExternalDependency(models.Model):
|
||||||
# Model for regression test of #11101
|
# Model for regression test of #11101
|
||||||
class Thingy(models.Model):
|
class Thingy(models.Model):
|
||||||
name = models.CharField(max_length=255)
|
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)
|
||||||
|
|
|
@ -7,6 +7,7 @@ import os
|
||||||
import re
|
import re
|
||||||
import warnings
|
import warnings
|
||||||
|
|
||||||
|
from django.core import serializers
|
||||||
from django.core.serializers.base import DeserializationError
|
from django.core.serializers.base import DeserializationError
|
||||||
from django.core import management
|
from django.core import management
|
||||||
from django.core.management.base import CommandError
|
from django.core.management.base import CommandError
|
||||||
|
@ -23,7 +24,12 @@ from django.utils.six import PY3, StringIO
|
||||||
|
|
||||||
from .models import (Animal, Stuff, Absolute, Parent, Child, Article, Widget,
|
from .models import (Animal, Stuff, Absolute, Parent, Child, Article, Widget,
|
||||||
Store, Person, Book, NKChild, RefToNKChild, Circle1, Circle2, Circle3,
|
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__)))
|
_cur_dir = os.path.dirname(os.path.abspath(upath(__file__)))
|
||||||
|
|
||||||
|
@ -702,6 +708,123 @@ class NaturalKeyFixtureTests(TestCase):
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
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):
|
class TestTicket11101(TransactionTestCase):
|
||||||
|
|
||||||
available_apps = [
|
available_apps = [
|
||||||
|
|
Loading…
Reference in New Issue