From a75324c6544d728d3bd8f678b1b8021fdff18332 Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 21 Aug 2013 03:21:21 +0300 Subject: [PATCH] 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. --- django/core/management/commands/dumpdata.py | 14 ++- tests/fixtures_regress/models.py | 94 +++++++++++++++ tests/fixtures_regress/tests.py | 125 +++++++++++++++++++- 3 files changed, 227 insertions(+), 6 deletions(-) diff --git a/django/core/management/commands/dumpdata.py b/django/core/management/commands/dumpdata.py index c54ccc1c587..b28ad590838 100644 --- a/django/core/management/commands/dumpdata.py +++ b/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() diff --git a/tests/fixtures_regress/models.py b/tests/fixtures_regress/models.py index 95f9488ab75..d8e497eee22 100644 --- a/tests/fixtures_regress/models.py +++ b/tests/fixtures_regress/models.py @@ -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) diff --git a/tests/fixtures_regress/tests.py b/tests/fixtures_regress/tests.py index 82929cfcb94..e9aca4252b2 100644 --- a/tests/fixtures_regress/tests.py +++ b/tests/fixtures_regress/tests.py @@ -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(), [ + "", + "" + ], ordered=False) + + class TestTicket11101(TransactionTestCase): available_apps = [