mirror of
https://github.com/django/django.git
synced 2024-12-01 15:42:04 +01:00
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:
|
||||
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()
|
||||
|
@ -235,3 +235,97 @@ class ExternalDependency(models.Model):
|
||||
# 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)
|
||||
|
@ -7,6 +7,7 @@ import os
|
||||
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 django.utils.six import PY3, StringIO
|
||||
|
||||
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 @@ 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):
|
||||
|
||||
available_apps = [
|
||||
|
Loading…
Reference in New Issue
Block a user