Opened 14 years ago

Closed 6 years ago

#14642 closed Bug (fixed)

save_as=True and generic inline in admin gives IndexError

Reported by: simon@… Owned by: nobody
Component: contrib.contenttypes Version: 1.4
Severity: Normal Keywords: save_as generic inline
Cc: Brillgen Developers, Paul Oswald, Patrick Kranzlmueller, lorin, Patrick K Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a model with a generic relation to another as below.

# models.py
class GenericObj(models.Model):
    content_type = models.ForeignKey(ContentType)
    object_id = models.PositiveIntegerField()
    content_object = generic.GenericForeignKey()

    text = models.CharField(max_length=50)


class ToObj(models.Model):
    text = models.CharField(max_length=50)

    obj_set = generic.GenericRelation(GenericObj)

In admin i have a form with an generic inline and save_as=True as below

# admin.py
class GenObjInline(generic.GenericTabularInline):
    model = GenericObj

class ToObjAdmin(admin.ModelAdmin):
    inlines = [GenObjInline, ]
    save_as = True

admin.site.register(ToObj, ToObjAdmin)

when i try to save as new in admin, i get an IndexError, list index out of range.

Traceback:

Environment:

Request Method: POST
Request URL: http://127.0.0.1:8000/admin/tsave/toobj/1/
Django Version: 1.2.3
Python Version: 2.6.4
Installed Applications:
['django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.sites',
 'django.contrib.messages',
 'tsave',
 'django.contrib.admin']
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')


Traceback:
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/core/handlers/base.py" in get_response
  100.                     response = callback(request, *callback_args, **callback_kwargs)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/contrib/admin/options.py" in wrapper
  239.                 return self.admin_site.admin_view(view)(*args, **kwargs)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/utils/decorators.py" in _wrapped_view
  76.                     response = view_func(request, *args, **kwargs)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  69.         response = view_func(request, *args, **kwargs)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/contrib/admin/sites.py" in inner
  190.             return view(request, *args, **kwargs)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/utils/decorators.py" in _wrapper
  21.             return decorator(bound_func)(*args, **kwargs)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/utils/decorators.py" in _wrapped_view
  76.                     response = view_func(request, *args, **kwargs)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/utils/decorators.py" in bound_func
  17.                 return func(self, *args2, **kwargs2)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/db/transaction.py" in _commit_on_success
  299.                     res = func(*args, **kw)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/contrib/admin/options.py" in change_view
  869.             return self.add_view(request, form_url='../add/')
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/utils/decorators.py" in _wrapper
  21.             return decorator(bound_func)(*args, **kwargs)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/utils/decorators.py" in _wrapped_view
  76.                     response = view_func(request, *args, **kwargs)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/utils/decorators.py" in bound_func
  17.                 return func(self, *args2, **kwargs2)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/db/transaction.py" in _commit_on_success
  299.                     res = func(*args, **kw)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/contrib/admin/options.py" in add_view
  792.                                   prefix=prefix, queryset=inline.queryset(request))
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/contrib/contenttypes/generic.py" in __init__
  318.             prefix=prefix
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/forms/models.py" in __init__
  429.         super(BaseModelFormSet, self).__init__(**defaults)
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/forms/formsets.py" in __init__
  47.         self._construct_forms()
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/forms/formsets.py" in _construct_forms
  97.             self.forms.append(self._construct_form(i))
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/forms/models.py" in _construct_form
  453.             kwargs['instance'] = self.get_queryset()[i]
File "/media/filesrv/simon/hostit/envs/django1.2/lib/python2.6/site-packages/django/db/models/query.py" in __getitem__
  172.             return self._result_cache[k]

Exception Type: IndexError at /admin/tsave/toobj/1/
Exception Value: list index out of range

Attachments (1)

inline-formset-test.diff (6.1 KB ) - added by Paul Oswald 13 years ago.

Download all attachments as: .zip

Change History (27)

comment:1 by cmarrer, 14 years ago

milestone: 1.3
Triage Stage: UnreviewedAccepted

comment:2 by JohnRandom, 14 years ago

Owner: changed from nobody to JohnRandom

comment:3 by JohnRandom, 14 years ago

Component: django.contrib.adminForms
Owner: changed from JohnRandom to nobody

I was able to reproduce the bug, but I'm not really sure that the classification 'django.contrib.admin' is the right one. I think it is more likely found in 'django.forms' or 'django.contrib.contenttypes'. I found that the new class is created but the transaction is then rolled back for (to me) unknown reason. I suspect that the children for the generic relation can't be found.

comment:4 by Brillgen Developers, 13 years ago

Cc: Brillgen Developers added

I have the same issue and I can confirm that the these two conditions will reproduce the bug in Django 1.0 as well as 1.2. Has any progress been made since?
JohnRandom, do you need help with this to meet the 1.3 deadline?

comment:5 by Loststylus, 13 years ago

I confirm, that i experience this problem, too, but I experience it not in the admin site, but with my application, whre I use ModelFormsets

comment:6 by Loststylus, 13 years ago

Are there any workarounds, yet?

comment:7 by Henrique C. Alves, 13 years ago

I confirm the same happens when using ModelFormSets in Django 1.2 and memcached backend (I don't know if it's relevant). The traceback looks the same as the posted above. Sometimes, but only sometimes, it'll throw an IndexError when saving a formset. This bug was proved hard to reproduce and I only have it in production.

Maybe we should change the ticket summary to "ModelFormSet throws IndexError"?

comment:8 by Loststylus, 13 years ago

Btw, yes, it's very hard to reproduce :(

comment:9 by Paul Oswald, 13 years ago

Cc: Paul Oswald added
Summary: save_as=True and generic inline in admin gives IndexErrorIndexError: list index out of range caused by inline formsets

I believe I have a fully reproduceable version of this issue. For me it comes when there is discrepancy between the form submitted and the database. I was getting this error in my production system and I thought it was my forms. Turns out I could eliminate my forms as a source of the issue by using the admin. I've changed the summary to match. I reduced it down to this test case in Django 1.2.4:

models.py:

class Thingy(models.Model):
    description = models.CharField(max_length=256)

class ThingyItem(models.Model):
    thingy = models.ForeignKey(Thingy)
    description = models.CharField(max_length=256)

admin.py:

class ThingyItemInline(admin.TabularInline):
    model = ThingyItem
    extra = 0

class ThingyAdmin(admin.ModelAdmin):
    inlines = [ThingyItemInline,]

admin.site.register(Thingy, ThingyAdmin)
admin.site.register(ThingyItem)

Now do the following:

  • Create a new Thingy with several ThingyItems in the admin and save it.
  • Open the edit page.
  • Open the edit page for the same thingy in a second browser window.
  • Check the "Delete" button on the last ThingyItem and save it in the second window.
  • Now go back to the first form and save it

When I do this, I get:

Traceback:
File "/Users/poswald/.virtualenvs/hats/lib/python2.6/site-packages/django/core/handlers/base.py" in get_response
  100.                     response = callback(request, *callback_args, **callback_kwargs)

... snipped because it's the same as above ...

File "/Users/poswald/.virtualenvs/hats/lib/python2.6/site-packages/django/db/models/query.py" in __getitem__
  171.             return self._result_cache[k]

Exception Type: IndexError at /admin/exampletest/thingy/1/unhandled
Exception Value: list index out of range

It seems that the inline formset code is a bit brittle. An error is thrown when the data submitted in the management form is not in agreement with the state of the database. I'd like to make an automated test case for this but I'm not sure how to do that sequence of steps in the test client. If anyone can shed some light onto what the formset code is *supposed* to do in this situation it would be useful. I imagine the options are the second form overrides the first or something like a form validation error is raised that indicates the form is stale.

by Paul Oswald, 13 years ago

Attachment: inline-formset-test.diff added

comment:10 by Paul Oswald, 13 years ago

It has been brought to my attention that I may have stolen this issue and perhaps my addition to this report is caused by an issue that is different than the original report. I tried to recreate the original issue using the models originally reported and was only able to reproduce it by following the steps I outlined above. Because of that, and that other people are reporting the problem to be intermittent, I believe they are the same issue and the underlying cause is the same. If someone else can confirm that the original issue occurs in a way other than expressed by this test case, I will split this out into it's own issue.

I attached a test to express this exception in the latest version of code. We need a design decision on what the proper handling of this case should be so that it can be coded up. I started that discussion here: http://groups.google.com/group/django-developers/browse_thread/thread/8c66461f1712dbb9#

comment:11 by Paul Oswald, 13 years ago

Summary: IndexError: list index out of range caused by inline formsetssave_as=True and generic inline in admin gives IndexError

It seems that this issue is reproducible as is. I've therefore created a new ticket (#15574) for tracking the issue I was seeing and reverting the subject of this one. I'll look into creating a test case for this issue as well.

comment:12 by Brillgen Developers, 13 years ago

The issue can be confirmed as occurring as is with exactly the steps described by the original reporter and occurs at every instance not intermittently.
I have been able to reproduce this with a fresh Django 1.2 install with no other models.

Is this at all on the radar for 1.3?

comment:13 by Russell Keith-Magee, 13 years ago

@brillgen - No, it isn't. 1.3 has been a release candidate for about a week, and will be finalized early next week. At this point,the only bugs being fixed are those that can be shown to cause catastrophic data loss, are regressions from 1.2, or are major problems with features added in the 1.3 cycle.

comment:14 by Brillgen Developers, 13 years ago

milestone: 1.3

I'm removing the milestone as 1.3 since 1.3 has come and gone. the bug owner (if any) can plan and update accordingly.

comment:15 by Julien Phalip, 13 years ago

Severity: Normal
Type: Bug

comment:16 by Patrick Kranzlmueller, 13 years ago

Cc: Patrick Kranzlmueller added
Easy pickings: unset
UI/UX: unset

comment:17 by anonymous, 12 years ago

Version: 1.21.3

comment:18 by lorin, 12 years ago

Cc: lorin added

comment:19 by lorin, 12 years ago

Version: 1.31.4

comment:20 by andrschwartz@…, 11 years ago

I think the problem is in wrong initial forms count calculation. So i've changed method

def initial_form_count(self):
        """Returns the number of forms that are required in this FormSet."""
        if self.is_bound:
            return self.management_form.cleaned_data[INITIAL_FORM_COUNT]
        else:
            # Use the length of the inital data if it's there, 0 otherwise.
            initial_forms = self.initial and len(self.initial) or 0
            if initial_forms > self.max_num >= 0:
                initial_forms = self.max_num
        return initial_forms

to ignore posted -INITIAL-FORMS if "Save as New" used:

def initial_form_count(self):
        """Returns the number of forms that are required in this FormSet."""
        if self.is_bound and not self.save_as_new:
            return self.management_form.cleaned_data[INITIAL_FORM_COUNT]
        else:
            # Use the length of the inital data if it's there, 0 otherwise.
            initial_forms = self.initial and len(self.initial) or 0
            if initial_forms > self.max_num >= 0:
                initial_forms = self.max_num
        return initial_forms

Works good for me. But i'm not sure this fix will work correctly in all situations.
See also BaseInlineFormSet initial_form_count method.

comment:21 by Patrick K, 11 years ago

Cc: Patrick K added

comment:22 by Patrick K, 11 years ago

anyone working on this one? the last mentioned patch does not solve it for me.

comment:23 by Tim Graham, 8 years ago

#26901 is a duplicate that proposes a (perhaps partial) patch.

comment:24 by Tim Graham, 6 years ago

Component: Formscontrib.contenttypes
Has patch: set
Patch needs improvement: set

PR (from duplicate ticket #28840) with a couple comments for improvement.

comment:25 by Tomer Chachamu, 6 years ago

Patch needs improvement: unset

comment:26 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: newclosed

In 9bc4d90:

Fixed #14642 -- Fixed generic inline formsets crash when using save_as_new=True.

Note: See TracTickets for help on using tickets.
Back to Top