[Django] #18510: Options._fill_related_objects_cache should assert for app_cache_ready() or deal with app_cache_ready() == False
Django
deal with app_cache_ready() == False
----------------------------------------------+--------------------
Reporter: vanschelven | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
It is possible to end up in Options._fill_related_many_to_many_cache
(https://github.com/django/django/blob/1.4/django/db/models/options.py#L388)
while app_cache_ready() is False. Since that method iterates over
get_models() (line 401) and saves its result in an object attribute
regardless of the value of app_cache_ready() in line 409 this may result
in the _related_objects_cache missing elements, causing get_field_by_name
(and possibly other methods) to fail.
Steps to reproduce:
{{{
# models.py
from django.db import models
from django.contrib.auth.models import User
User._meta.get_field_by_name('username')
# alternatively, import the below to trigger the error.
# from django.contrib.auth.admin import UserAdmin
class UserProfile(models.Model):
user = models.ForeignKey(User, unique=True, editable=False)
first_name = models.CharField(max_length=255, blank=True)
User._meta.ordering = ('userprofile__first_name',)
}}}
{{{
# tests.py
from django.test import TestCase
from django.contrib.auth.models import User
class SimpleTest(TestCase):
def test_get_field_by_name(self):
# no assertions needed, the test simply fails
User.objects.create(username='test')
User._meta.get_field_by_name('userprofile')
# similarly methods using get_field_by_name themselves fail, such
as
u = User.objects.all()[0]
}}}
{{{
----------------------------------------------------------------------
Traceback (most recent call last):
File "/tmp/ve13/reverselookup/userprofile/tests.py", line 9, in
test_get_field_by_name
User._meta.get_field_by_name('userprofile')
File "/tmp/ve14/lib/python2.6/site-
packages/django/db/models/options.py", line 315, in get_field_by_name
% (self.object_name, name))
!FieldDoesNotExist: User has no field named 'userprofile'
}}}
This fails in both Django 1.3 and Django 1.4.
Solution:
The canonical solution in options.py seems to be to check for the value of
app_cache_ready() before actually setting the internal cache attribute.
E.g.
https://github.com/django/django/blob/1.4/django/db/models/options.py#L447
. I'm not sure whether that would fix all problems caused by this or
whether an assert for app_cache_ready() at the top of the method would be
more appropriate.
I'm currently not able to come up with a proper way of providing tests for
the situation above, because the global state of app loading and such
makes coming up with tests somewhat harder. If anyone can help me with
that, that would be great. Alternatively perhaps someone is willing to
take the above method of reproduction and the fact that no old tests break
as enough reason for a patch?
Admittedly the above scenario appears contrived (calling
User._meta.get_field_by_name('username') in a models.py file). However,
this is simply the easiest way to reproduce the problem.
I ran into this when further debugging #18507 . In that case I was using
from django.contrib.auth.admin import !UserAdmin somewhere early in one of
my models files. Since Django 1.3 adds more validation to the admins, one
of which contained a call to get_field_by_name this resulted in a rather
hard-to-debug problem when upgrading from Django 1.2 to Django 1.3 (or
1.4).
--
Ticket URL: <https://code.djangoproject.com/ticket/18510>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
deal with app_cache_ready() == False
Type: Uncategorized | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by vanschelven):
* cc: vanschelven (added)
* needs_better_patch: => 0
* has_patch: 0 => 1
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/18510#comment:1>
Django
deal with app_cache_ready() == False
Reporter: vanschelven | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/18510#comment:2>
Django
deal with app_cache_ready() == False
Reporter: vanschelven | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 1.4
Severity: Normal | Triage Stage: Accepted
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* keywords: => app-loading
* status: new => closed
* resolution: => fixed
Comment:
This is "fixed" by the app-loading refactor, in the sense that Django now
throws an explicit exception.
'''stable/1.6.x -- obscure test failure'''
{{{
Creating test database for alias 'default'...
E
======================================================================
ERROR: test_get_field_by_name (testapp.tests.SimpleTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "~/django/django/db/models/options.py", line 371, in
get_field_by_name
return self._name_map[name]
KeyError: 'userprofile'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "~/django/t18510/testapp/tests.py", line 10, in
test_get_field_by_name
User._meta.get_field_by_name('userprofile')
File "~/django/django/db/models/options.py", line 377, in
get_field_by_name
% (self.object_name, name))
django.db.models.fields.FieldDoesNotExist: User has no field named
'userprofile'
----------------------------------------------------------------------
Ran 1 test in 0.002s
FAILED (errors=1)
Destroying test database for alias 'default'...
}}}
'''stable/1.7.x -- explicit exception'''
{{{
Traceback (most recent call last):
File "~/django/django/db/models/options.py", line 414, in
get_field_by_name
return self._name_map[name]
AttributeError: 'Options' object has no attribute '_name_map'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "~/django/django/db/models/options.py", line 559, in
get_all_related_m2m_objects_with_model
cache = self._related_many_to_many_cache
AttributeError: 'Options' object has no attribute
'_related_many_to_many_cache'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "./manage.py", line 10, in <module>
execute_from_command_line(sys.argv)
File "~/django/django/core/management/__init__.py", line 385, in
execute_from_command_line
utility.execute()
File "~/django/django/core/management/__init__.py", line 354, in execute
django.setup()
File "~/django/django/__init__.py", line 21, in setup
apps.populate(settings.INSTALLED_APPS)
File "~/django/django/apps/registry.py", line 106, in populate
app_config.import_models(all_models)
File "~/django/django/apps/config.py", line 190, in import_models
self.models_module = import_module(models_module_name)
File "~/.virtualenvs/django/lib/python3.4/importlib/__init__.py", line
104, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "<frozen importlib._bootstrap>", line 2231, in _gcd_import
File "<frozen importlib._bootstrap>", line 2214, in _find_and_load
File "<frozen importlib._bootstrap>", line 2203, in
_find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 1200, in _load_unlocked
File "<frozen importlib._bootstrap>", line 1129, in _exec
File "<frozen importlib._bootstrap>", line 1448, in exec_module
File "<frozen importlib._bootstrap>", line 321, in
_call_with_frames_removed
File "~/django/t18510/testapp/models.py", line 5, in <module>
User._meta.get_field_by_name('username')
File "~/django/django/db/models/options.py", line 416, in
get_field_by_name
cache = self.init_name_map()
File "~/django/django/db/models/options.py", line 445, in init_name_map
for f, model in self.get_all_related_m2m_objects_with_model():
File "~/django/django/db/models/options.py", line 561, in
get_all_related_m2m_objects_with_model
cache = self._fill_related_many_to_many_cache()
File "~/django/django/db/models/options.py", line 575, in
_fill_related_many_to_many_cache
for klass in self.apps.get_models():
File "~/.virtualenvs/django/lib/python3.4/functools.py", line 428, in
wrapper
result = user_function(*args, **kwds)
File "~/django/django/apps/registry.py", line 156, in get_models
self.check_ready()
File "~/django/django/apps/registry.py", line 119, in check_ready
raise RuntimeError("App registry isn't ready yet.")
RuntimeError: App registry isn't ready yet.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18510#comment:3>