Code

Opened 3 years ago

Last modified 7 months ago

#14642 new Bug

save_as=True and generic inline in admin gives IndexError

Reported by: simon@… Owned by: nobody
Component: Forms Version: 1.4
Severity: Normal Keywords: save_as generic inline
Cc: brillgen, poswald, patrickk, lorin, sehmaschine Triage Stage: Accepted
Has patch: no 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 poswald 3 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 3 years ago by cmarrer

  • milestone set to 1.3
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 3 years ago by JohnRandom

  • Owner changed from nobody to JohnRandom

comment:3 Changed 3 years ago by JohnRandom

  • Component changed from django.contrib.admin to Forms
  • 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 Changed 3 years ago by brillgen

  • Cc brillgen 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 Changed 3 years ago by Loststylus

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 Changed 3 years ago by Loststylus

Are there any workarounds, yet?

comment:7 Changed 3 years ago by hcarvalhoalves

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 Changed 3 years ago by Loststylus

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

comment:9 Changed 3 years ago by poswald

  • Cc poswald added
  • Summary changed from save_as=True and generic inline in admin gives IndexError to IndexError: 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.

Changed 3 years ago by poswald

comment:10 Changed 3 years ago by poswald

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 Changed 3 years ago by poswald

  • Summary changed from IndexError: list index out of range caused by inline formsets to save_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 Changed 3 years ago by brillgen

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 Changed 3 years ago by russellm

@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 Changed 3 years ago by brillgen

  • milestone 1.3 deleted

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 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:16 Changed 3 years ago by patrickk

  • Cc patrickk added
  • Easy pickings unset
  • UI/UX unset

comment:17 Changed 2 years ago by anonymous

  • Version changed from 1.2 to 1.3

comment:18 Changed 2 years ago by lorin

  • Cc lorin added

comment:19 Changed 2 years ago by lorin

  • Version changed from 1.3 to 1.4

comment:20 Changed 17 months ago by andrschwartz@…

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 Changed 7 months ago by sehmaschine

  • Cc sehmaschine added

comment:22 Changed 7 months ago by sehmaschine

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.