When extending from abstract model, ForeignKey points to wrong application

[skyjur]

This might be related to https://code.djangoproject.com/ticket/25858

I have following code:

app_A/abstract.py:

class AbstractModel1(Model):
    class Meta:
        abstract = True


class AbstractModel2(Model):
    model1 = ForeignKey('Model1')

    class Meta:
          abstract = True

app_A/models.py:

from .abstract import AbstractModel1, AbstractModel12

class Model1(AbstractModel1):
    pass

class Model2(AbstractModel12):
    pass

app_B/models.py:

from app_A.abstract import AbstractModel1, AbstractModel2

class Model1(AbstractModel1):
    pass

class Model2(AbstractModel12):
    pass

in Django 1.8, the app_B.Model2.model1 would point to app_B.Model1, but in Django 1.9.2 it points to app_A.Model1 so my code no longer works.

Test case can be found: ​https://github.com/skyjur/django-ticketing/tree/master/ticket_26186

[Simon Charette]

Hi skyjur,

While I can reproduce on Django 1.9 and 1.9.1 (where it was considered a regression #25858) I cannot reproduce on any version of Django 1.8.

I even tried swapping the order of 'app_A' and 'app_B' in INSTALLED_APPS with no success.

You can find the test applications I used in my attempts to reproduce ​here.

Are you sure you managed to reproduce against Django 1.8? If it's the case please reopen this ticket with more details.

[skyjur]

I have updated your test case:
​https://github.com/charettes/django-ticketing/pull/1

In my app my abstract models are defined in a/abstract.py and then imported into a/models.py, I didn't realise this was important.

[skyjur]

I have added another test case showing that in 1.9 it's impossible to extend from abstract models if they contain foreign keys to abstract model that is not inside an installed app.

In 1.8 this was possible:

​https://github.com/skyjur/django-ticketing/tree/master/ticket_26186_2

tox output:

django189 runtests: commands[0] | django-admin test
Creating test database for alias 'default'...
.
----------------------------------------------------------------------
Ran 1 test in 0.000s

OK
Destroying test database for alias 'default'...
django192 runtests: commands[0] | django-admin test
Creating test database for alias 'default'...
Traceback (most recent call last):
    <...>
    self._related_fields = self.resolve_related_fields()
  File "django-ticketing/ticket_26186/.tox/django192/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 567, in resolve_related_fields
    raise ValueError('Related model %r cannot be resolved' % self.remote_field.model)
ValueError: Related model u'None.Model1' cannot be resolved
ERROR: InvocationError: 'django-ticketing/ticket_26186_2/.tox/django192/bin/django-admin test'
ERROR: InvocationError: 'django-ticketing/ticket_26186_2/.tox/django192/bin/django-admin test'
______________summary__________________________ __________
  django189: commands succeeded
ERROR:   django192: commands failed

[Tim Graham]

Does it raise a deprecation warning in 1.8? There was a deprecation that ended in 1.9that may be relevant -- from the release notes: "All models need to be defined inside an installed application or declare an explicit app_label." To confirm this, can you bisect to the commit in Django where the behavior changed?

[Simon Charette]

In your example both app_B.Model2.model1 and app_A.Model2.model1 are pointing to app_B.Model1 because app_B's models module was imported first (because of the order of INSTALLED_APPS) and triggered the import of app_A.abstract.

No version of Django allowed you to make Model2 point to their respective Model1. The only thing that changed in Django 1.9.2 is that lazy related references defined on abstract models are now bound to the application in which they are defined.

[Simon Charette]

Well I stand corrected. It looks like the reported behavior is possible as long as the application containing the abstract module is not yet imported.

Here's what happens on Django 1.8.

The b application is imported by the app loading mechanism and triggers and the import of a.abstract. At this point the abstract models are not bound to any application and thus the AbstractModel2.model1 field is not resolved to any model which allows the b.Model2.model1 field to resolves to b.Model1. When the a application is loaded the abstract models are bound to it (because they are defined in a a submodule) and thus a.Model2 automatically resolves to a.Model1.

In this case if the a application is loaded before the b application b.Model2 will point to a.Model1 because the abstract models were bound to the a application when b.models will import a.abstract (which was imported by a.models first).

Here's what happen before #25858 was fixed (1.9, 1.9.1):

App relative lazy relationships defined on abstract models were never resolved thus the behavior described above if the b application appeared before the a one on Django 1.8 was always present and not dependent on ÌNSTALLED_APPS ordering.

Here's what happen since #25858 is fixed (1.9.2 , master):

It's not possible anymore as app relative lazy relationships defined on abstract models are bound to the application in which they are defined.

I'll take this to the ML again to gather feedback but I'm inclined to close this as wontfix because the behavior was only achievable in 1.8 through import side effects.

[Simon Charette]

Posted to the ​mailing list.

I suggested we revert the fix for #25858 and instead document how app relative relationships defined on
abstract models are resolved.

[Simon Charette]

Here's a ​PR with the suggested changes.

[Tim Graham]

Left comments for improvement.

[Simon Charette <charette.s@…>]

In 0223e213:

Fixed #26186 -- Documented how app relative relationships of abstract models behave.

This partially reverts commit bc7d201bdbaeac14a49f51a9ef292d6312b4c45e.

Thanks Tim for the review.

Refs #25858.

[Simon Charette <charette.s@…>]

In 48cf7516:

[1.9.x] Fixed #26186 -- Documented how app relative relationships of abstract models behave.

This partially reverts commit bc7d201bdbaeac14a49f51a9ef292d6312b4c45e.

Thanks Tim for the review.

Refs #25858.

Backport of 0223e213dd690b6b6e0669f836a20efb10998c83 from master