[Django] #19524: DoesNotExist exception when adding object in admin with inline object
Django
-------------------------------+----------------------------------------
Reporter: qcwxezdas@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.5-beta-1
Severity: Normal | Keywords: admin,DoesNotExist,inlines
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+----------------------------------------
'''models.py'''
{{{
class Service(models.Model):
provider = models.ForeignKey("services.ServiceProvider",
verbose_name=_("Service provider"),
related_name="services")
title = models.CharField(_("Title"), max_length=150)
# ... other fields ...
class EntertainmentType(models.Model):
title = models.CharField(max_length=150, unique=True)
def __unicode__(self):
return self.title
class Meta:
ordering = ('title',)
class Entertainment(Service):
types = models.ManyToManyField(EntertainmentType,
verbose_name=_("Types"),
related_name="entertainments")
description = models.TextField(_("Description"), blank=True)
# ... other fields ...
def __unicode__(self):
return self.title
class Meta:
ordering = ('title',)
class EntertainmentPhoto(models.Model):
entertainment = models.ForeignKey(Entertainment,
related_name="photos")
description = models.TextField(_("Description"), max_length=1000,
blank=True)
photo = ImageWithThumbsField(_("Photo"), sizes=(),
upload_to="services/photos/entertainment")
is_main = models.BooleanField(verbose_name=_("Main"), default=False)
class Meta:
ordering = ('-is_main',)
verbose_name = _("Photo")
verbose_name_plural = _("Photos")
}}}
'''admin.py'''
{{{
from django.contrib import admin
from services.entertainment.models import Entertainment,
EntertainmentType, EntertainmentPhoto
class EntertainmentTypeAdmin(admin.ModelAdmin):
pass
class EntertainmentPhotoInline(admin.TabularInline):
model = EntertainmentPhoto
class EntertainmentAdmin(admin.ModelAdmin):
filter_horizontal = ["types"]
list_display = Entertainment.list_display
list_filter = ["provider", "types", ]
inlines = [EntertainmentPhotoInline]
admin.site.register(Entertainment, EntertainmentAdmin)
admin.site.register(EntertainmentType, EntertainmentTypeAdmin)
}}}
If I add the Entertainment object without any EntertainmentPhoto inlines
then everything wents correct. I am ABLE to add inline photos when editing
created instance.
However, if I add photos when instance is not created (in add view), I get
the following:
{{{
Environment:
Request Method: POST
Request URL:
http://77.79.59.184:8000/admin/entertainment/entertainment/add/?provider=1
Django Version: 1.5b2
Python Version: 2.7.2
Installed Applications:
['south',
'gunicorn',
'django_extensions',
'django.contrib.databrowse',
'django.contrib.contenttypes',
'django.contrib.sessions',
'django.contrib.sites',
'django.contrib.messages',
'django.contrib.staticfiles',
'django.contrib.admin',
'authentication',
'framework',
'framework.menu',
'services',
'maps',
'services.entertainment',
'services.otherservices']
Installed Middleware:
('django.middleware.common.CommonMiddleware',
'django.contrib.sessions.middleware.SessionMiddleware',
'django.middleware.csrf.CsrfViewMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
'django.middleware.clickjacking.XFrameOptionsMiddleware')
Traceback:
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/core/handlers/base.py" in get_response
116. response = callback(request,
*callback_args, **callback_kwargs)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/contrib/admin/options.py" in wrapper
370. return self.admin_site.admin_view(view)(*args,
**kwargs)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/utils/decorators.py" in _wrapped_view
91. response = view_func(request, *args, **kwargs)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/views/decorators/cache.py" in _wrapped_view_func
89. response = view_func(request, *args, **kwargs)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/contrib/admin/sites.py" in inner
202. return view(request, *args, **kwargs)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/utils/decorators.py" in _wrapper
25. return bound_func(*args, **kwargs)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/utils/decorators.py" in _wrapped_view
91. response = view_func(request, *args, **kwargs)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/utils/decorators.py" in bound_func
21. return func(self, *args2, **kwargs2)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/db/transaction.py" in inner
208. return func(*args, **kwargs)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/contrib/admin/options.py" in add_view
1045. self.save_related(request, form, formsets, False)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/contrib/admin/options.py" in save_related
762. self.save_formset(request, form, formset,
change=change)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/contrib/admin/options.py" in save_formset
750. formset.save()
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/forms/models.py" in save
494. return self.save_existing_objects(commit) +
self.save_new_objects(commit)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/forms/models.py" in save_new_objects
628. self.new_objects.append(self.save_new(form,
commit=commit))
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/forms/models.py" in save_new
728. pk_value = getattr(self.instance, self.fk.rel.field_name)
File "/Users/aemdy/virtualenvs/django1.5/lib/python2.7/site-
packages/django/db/models/fields/related.py" in __get__
389. raise self.field.rel.to.DoesNotExist
Exception Type: DoesNotExist at /admin/entertainment/entertainment/add/
Exception Value:
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19524>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: qcwxezdas@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version:
Keywords: | Resolution:
admin,DoesNotExist,inlines | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: => 0
* needs_docs: => 0
* severity: Normal => Release blocker
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
I've been able to reproduce the bug, which does not happen on 1.4.x
branch. Bisecting the code, the faulty commit seems to be
[b90d4e5b749be3b].
--
Ticket URL: <https://code.djangoproject.com/ticket/19524#comment:1>
Django
Reporter: qcwxezdas@… | Owner: aaugustin
Type: Bug | Status: assigned
Component: contrib.admin | Version:
Severity: Release blocker | 1.5-beta-1
Keywords: | Resolution:
admin,DoesNotExist,inlines | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* owner: nobody => aaugustin
* status: new => assigned
Comment:
Most likely the admin was relying on the ORM incorrectly returning `None`
instead raising `DoesNotExist`.
--
Ticket URL: <https://code.djangoproject.com/ticket/19524#comment:2>
Django
Reporter: qcwxezdas@… | Owner: aaugustin
Type: Bug | Status: assigned
Component: contrib.admin | Version:
Severity: Release blocker | 1.5-beta-1
Keywords: | Resolution:
admin,DoesNotExist,inlines | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
The bug happens when the parent model inherits another model.
I'm attaching a minimal failing test case.
--
Ticket URL: <https://code.djangoproject.com/ticket/19524#comment:3>
Django
Reporter: qcwxezdas@… | Owner: aaugustin
Type: Bug | Status: assigned
Component: contrib.admin | Version:
Severity: Release blocker | 1.5-beta-1
Keywords: | Resolution:
admin,DoesNotExist,inlines | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
And here's a patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/19524#comment:4>
Django
Reporter: qcwxezdas@… | Owner: aaugustin
Type: Bug | Status: assigned
Component: contrib.admin | Version:
Severity: Release blocker | 1.5-beta-1
Keywords: | Resolution:
admin,DoesNotExist,inlines | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
I finally understood the reason for this regression.
Django has been caching related objects for a long time, but in an
inconsistent way. Among other fixes, during the 1.5 development cycle, I
added the ability to cache the absence of a related object. This is
necessary for `select_related` to operate correctly on nullable foreign
keys.
Unfortunately, I missed the case of unsaved objects. Before my changes,
caching never worked on unsaved objects: since their pk was `None`,
nothing was cached, and the relation was refreshed at each access, until
both objects were in the database and the relation could be correctly
loaded (and cached).
Now, the lack of a related object can be accidentally and mistakenly
cached before the objects are saved. Hence the spurious `DoesNotExist`
exception.
----
There are two approches to fix this problem:
1) Prevent the cache from being incorrectly populated in the first place:
- This problem only occurs when the programmer (or Django) is accessing
a relation that isn't saved to the database yet. That will return a wrong
result. Sometimes it doesn't matter; still, it's a programming mistake
that ought to be fixed.
- Here, the problem is triggered by `queryset.filter(**{self.fk.name:
self.instance})` is `BaseInlineFormSet.__init__`. This eventually calls
`RelatedField._pk_trace`, who follows the chain of related objects,
populating caches in the process. Calling `.filter(xxx=unsaved_instance)`
is a programming error, but Django currently doesn't detect it (see
#17541).
2) Invalidate the cache when the relation is saved (in case it's been
accidentally populated):
- That's what my first patch does.
- It mixes concerns between the related object descriptors and the base
model class a bit :/
- I've tried to audit for other instances of this problem but it's very
difficult to achieve through code inspection.
Even though the first fix would be sufficient for this issue, I think the
second one is worth including for extra safety. It's just too easy to
accidentally populate the cache before saving an object, and it doesn't
cost much to invalidate at this point.
--
Ticket URL: <https://code.djangoproject.com/ticket/19524#comment:5>
Django
Reporter: qcwxezdas@… | Owner: aaugustin
Component: contrib.admin | Version:
Severity: Release blocker | 1.5-beta-1
admin,DoesNotExist,inlines | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aymeric Augustin <aymeric.augustin@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e9c24bef74e55729b190cf07e0ac452aa4c86fcd"]:
{{{
#!CommitTicketReference repository=""
revision="e9c24bef74e55729b190cf07e0ac452aa4c86fcd"
Fix #19524 -- Incorrect caching of parents of unsaved model instances.
Thanks qcwxezdas for the report. Refs #13839.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19524#comment:6>
Django
Reporter: qcwxezdas@… | Owner: aaugustin
Type: Bug | Status: closed
Component: contrib.admin | Version:
Severity: Release blocker | 1.5-beta-1
Keywords: | Resolution: fixed
admin,DoesNotExist,inlines | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"5097d3c5faab2b6582c4cebee2b265fcdbb893eb"]:
{{{
#!CommitTicketReference repository=""
revision="5097d3c5faab2b6582c4cebee2b265fcdbb893eb"
[1.5.x] Fix #19524 -- Incorrect caching of parents of unsaved model
instances.
Thanks qcwxezdas for the report. Refs #13839.
Backport of e9c24be.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19524#comment:7>
Django
Reporter: qcwxezdas@… | Owner: Aymeric
| Augustin
Type: Bug | Status: closed
Component: contrib.admin | Version:
| 1.5-beta-1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
admin,DoesNotExist,inlines |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"18245b948bf7032a0b50d92a743eff822f5bc6a6" 18245b94]:
{{{
#!CommitTicketReference repository=""
revision="18245b948bf7032a0b50d92a743eff822f5bc6a6"
Refs #7488, Refs #19524 -- Removed obsolete
ModelInheritanceTest.test_issue_7488() test.
Obsolete since e9c24bef74e55729b190cf07e0ac452aa4c86fcd.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19524#comment:8>