Opened 13 years ago
Closed 11 years ago
#18510 closed Bug (fixed)
Options._fill_related_objects_cache should assert for app_cache_ready() or deal with app_cache_ready() == False
| Reported by: | Klaas van Schelven | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 1.4 |
| Severity: | Normal | Keywords: | app-loading |
| Cc: | Klaas van Schelven | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
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).
Attachments (1)
Change History (4)
by , 13 years ago
comment:1 by , 13 years ago
| Cc: | added |
|---|---|
| Has patch: | set |
comment:2 by , 13 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|---|
| Type: | Uncategorized → Bug |
comment:3 by , 11 years ago
| Keywords: | app-loading added |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
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.