Data loss when ManyToManyField refers to the wrong table

[Ian Foote]

If I have two identically named models in two different apps, but these models each have a ManyToManyField to the same model, then one of these ManyToManyFields will use the other's database table.

App 1:

class Bar(models.Model):
    name = models.CharField(max_length=255)

class Foo(models.Model):
    name = models.CharField(max_length=255)
    bars = models.ManyToManyField(Bar, related_name='+')

App 2:

from app1.models import Bar

class Foo(models.Model):
   name = models.CharField(max_length=255)
   bars = models.ManyToManyField(Bar, related_name='+')

Tests:

from app1.models import Bar, Foo

def test_add_bar_to_foo():
    bar = Bar.objects.create()
    foo = Foo.objects.create()

    foo.bars.add(bar)

    assert bar in foo.bars.all()

I expect the test to pass, but instead it fails.

A working example of this code can be found at: ​https://github.com/Ian-Foote/django-bug-sandbox/pull/2

[Tim Graham]

Django 1.6 gave a validation error for this case:

CommandError: One or more models did not validate:
foo.foo: Accessor for m2m field 'bars' clashes with related m2m field 'Bar.+'. Add a related_name argument to the definition for 'bars'.
foo.foo: Reverse query name for m2m field 'bars' clashes with related m2m field 'Bar.+'. Add a related_name argument to the definition for 'bars'.
foo2.foo: Accessor for m2m field 'bars' clashes with related m2m field 'Bar.+'. Add a related_name argument to the definition for 'bars'.
foo2.foo: Reverse query name for m2m field 'bars' clashes with related m2m field 'Bar.+'. Add a related_name argument to the definition for 'bars'.

but this ceased after 9f76ea1eaad0db0183fa3b5bade16392db0cafbd.

This might be related to #24505 and its duplicate, #25486.

[Simon Charette]

I suspect this is caused by colliding related_query_names.

When you hide a related manager by setting related_name to a value ending with '+' the related_query_name is still set to its default value (based on the model name).

If we explicit the model definitions created by Django here's what we have:

class Bar(models.Model):
    class Meta:
        app_label = 'app1'

class Foo(models.Model):
    bars = models.ManyToManyField(Bar, related_name='+', through='app1.FooBar')

    class Meta:
        app_label = 'app1'

class FooBar(models.Model):
    bar = models.ForeignKey(
        Bar, related_name='+', related_query_name='foobar'
    )
    foo = models.ForeignKey(
        'app1.Foo', related_name='+', related_query_name='foobar'
    )

    class Meta:
        app_label = 'app1'

class Foo(models.Model):
    bars = models.ManyToManyField(Bar, related_name='+', through='app2.FooBar')

    class Meta:
        app_label = 'app2'

class FooBar(models.Model):
    bar = models.ForeignKey(
        Bar, related_name='+', related_query_name='foobar'
    )
    foo = models.ForeignKey(
        'app2.Foo', related_name='+', related_query_name='foobar'
    )

    class Meta:
        app_label = 'app2'

As you can see there's a related_query_name='foobar' conflict on Bar that makes foo.bars.all() (resolving to Bar.object.filter(foobar__foo=foo) ambiguous. This clash is not detected by the check framework because the ​relationships are hidden.

In order to fix this I suggest we start by fixing the clash checks to run even if the relationship is hidden, we just have to skip E302 and E304 in this case.

Then we should consider making implicit intermediary model creation more smart about this possible conflict by namespacing related_query_name by application label. This is a backward incompatible change in some way since users might have relied on the automatically generated queryname even if unlikely. The backward incompatibility could be lessened by only namespacing the queryname if the referenced model is from a different application.

[Simon Charette]

Here's a ​PR with the checks adjustments.

[Simon Charette <charette.s@…>]

In a325fb1f:

Fixed #26162 -- Checked query name clashes of hidden relationships.

Although reverse accessor clashes should be skipped query name can't be hidden.

Thanks to Ian Foote and Tim Graham for the review.

[Simon Charette <charette.s@…>]

In 5872372:

Fixed #26162 -- Checked query name clashes of hidden relationships.

Although reverse accessor clashes should be skipped query name can't be hidden.

Thanks to Ian Foote and Tim Graham for the review.

[Simon Charette <charette.s@…>]

In edff550:

[1.8.x] Fixed #26162 -- Checked query name clashes of hidden relationships.

Although reverse accessor clashes should be skipped query name can't be hidden.

Thanks to Ian Foote and Tim Graham for the review.

Backport of a325fb1f9b14b46288d0e1342407be4a6db2bdb1 from master