Opened 8 years ago

Closed 8 years ago

Last modified 5 years ago

#11116 closed (fixed)

delete() triggers an error on proxy model instances

Reported by: samuel.adam@… Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: proxy, model, delete
Cc: andy@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

>>> from hit.vote.models import VoteProxy

>>> inst = VoteProxy.objects.get(pk=2)

>>> inst.delete()
------------------------------------------------------------
Traceback (most recent call last):
  File "<ipython console>", line 1, in <module>
  File "/home/sam/projects/djangotrunk-env/lib/python2.5/site-packages/django/db/models/base.py", line 557, in delete
    self._collect_sub_objects(seen_objs)
  File "/home/sam/projects/djangotrunk-env/lib/python2.5/site-packages/django/db/models/base.py", line 544, in _collect_sub_objects
    parent_obj = getattr(self, link.name)
AttributeError: 'NoneType' object has no attribute 'name'

The problem is in django/db/models/base.py

http://code.djangoproject.com/browser/django/trunk/django/db/models/base.py#L151

151                     elif not is_proxy:
152                         attr_name = '%s_ptr' % base._meta.module_name
153                         field = OneToOneField(base, name=attr_name,
154                                 auto_created=True, parent_link=True)
155                         new_class.add_to_class(attr_name, field)
156                     else:
157                         field = None
158                     new_class._meta.parents[base] = field
>>> inst._meta.parents
{<class 'voting.models.Vote'>: None}

http://code.djangoproject.com/browser/django/trunk/django/db/models/base.py#L541

541             parent_stack = self._meta.parents.values()
542             while parent_stack:
543                 link = parent_stack.pop()
544                 parent_obj = getattr(self, link.name)
>>> inst._meta.parents.values()
[None]

Attachments (3)

11116_proxy_model_delete.diff (1.3 KB) - added by Paul McLanahan 8 years ago.
Possible fix
11116_proxy_model_delete2.diff (673 bytes) - added by samueladam 8 years ago.
Filtering parent.values() for True objects
11116_model_proxy_delete_with_test.diff (1.2 KB) - added by samueladam 8 years ago.
Added test

Download all attachments as: .zip

Change History (14)

comment:1 Changed 8 years ago by anonymous

Version: 1.0SVN

comment:2 Changed 8 years ago by Paul McLanahan

I'm having this exact problem. Bump.

Changed 8 years ago by Paul McLanahan

Possible fix

comment:3 Changed 8 years ago by Paul McLanahan

Has patch: set

Changed 8 years ago by samueladam

Filtering parent.values() for True objects

comment:4 Changed 8 years ago by samueladam

With 11116_proxy_model_delete2.diff:

Ran 829 tests in 388.585s

OK

comment:5 in reply to:  4 Changed 8 years ago by Karen Tracey

milestone: 1.1
Needs tests: set

Replying to samueladam

That's good the current tests pass, what about a test that shows the failure and shows that the patch fixes it? Neither patch has that, so far as I can see.

Setting 1.1 milestone since someone with a clue in this area of the code should probably look at this for 1.1.

Changed 8 years ago by samueladam

Added test

comment:6 Changed 8 years ago by samueladam

Needs tests: unset

@kmtracey

There you go for the test

@pmclanahan

Fox a fix with an "if", those are more pythonic:

if link:
    pass

#or

if hasattr(link, 'name'):
    pass

comment:7 Changed 8 years ago by Paul McLanahan

@samueladam

I agree. And I had that first, but then I thought about the fact that I was trying to correct for a specific condition where the value was set to None, and since I wasn't absolutely sure of the truth of typical values, I decided to be explicit, which I considered to be in keeping w/ the zen. But if expected values always evaluate to true, then you're right.

In any case, as long as we can get this fixed and checked in, I'll be happy. Thanks for the help!

comment:8 Changed 8 years ago by Paul McLanahan

Test fails w/ old code, passes with the new. Looks perfect. Thanks samueladam!

comment:9 Changed 8 years ago by anonymous

Cc: andy@… added

comment:10 Changed 8 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [10824]) Fixed #11116 -- Corrected the deletion of proxy objects. Thanks to Samuel Adam for the fix.

comment:11 Changed 5 years ago by Jacob

milestone: 1.1

Milestone 1.1 deleted

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