Opened 6 years ago

Closed 6 years ago

Last modified 4 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 pmclanahan 6 years ago.
Possible fix
11116_proxy_model_delete2.diff (673 bytes) - added by samueladam 6 years ago.
Filtering parent.values() for True objects
11116_model_proxy_delete_with_test.diff (1.2 KB) - added by samueladam 6 years ago.
Added test

Download all attachments as: .zip

Change History (14)

comment:1 Changed 6 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Version changed from 1.0 to SVN

comment:2 Changed 6 years ago by pmclanahan

I'm having this exact problem. Bump.

Changed 6 years ago by pmclanahan

Possible fix

comment:3 Changed 6 years ago by pmclanahan

  • Has patch set

Changed 6 years ago by samueladam

Filtering parent.values() for True objects

comment:4 follow-up: Changed 6 years ago by samueladam

With 11116_proxy_model_delete2.diff:

Ran 829 tests in 388.585s

OK

comment:5 in reply to: ↑ 4 Changed 6 years ago by kmtracey

  • milestone set to 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 6 years ago by samueladam

Added test

comment:6 Changed 6 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 6 years ago by pmclanahan

@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 6 years ago by pmclanahan

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

comment:9 Changed 6 years ago by anonymous

  • Cc andy@… added

comment:10 Changed 6 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:11 Changed 4 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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