Opened 12 years ago

Closed 12 years ago

#17717 closed Bug (fixed)

django.core.serializers.serialize doesn't deal with proxy models

Reported by: lakin Owned by: nobody
Component: Core (Serialization) Version: 1.1
Severity: Normal Keywords:
Cc: anssi.kaariainen@…, charette.s@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If you try to serialize a proxy=True model - you get no fields serialized.

I wish I had more time to make a test case, but I don't at the moment. The easy test case is to make a model, make another model that proxies to the original model. Insert a row of data, and then serialize an instance of both models to see that the proxy one is empty.

Marking it as 1.1 as that is what I'm using and I haven't had time to double check the other versions.

Attachments (3)

17717-test.diff (2.1 KB ) - added by Claude Paroz 12 years ago.
Test showing bug
17717.diff (3.5 KB ) - added by Anssi Kääriäinen 12 years ago.
ticket-17717-proxy-model-serializers.diff (3.6 KB ) - added by Simon Charette 12 years ago.
Make sure to rely on newly introduced concrete_model model option

Download all attachments as: .zip

Change History (12)

by Claude Paroz, 12 years ago

Attachment: 17717-test.diff added

Test showing bug

comment:1 by Claude Paroz, 12 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Anssi Kääriäinen, 12 years ago

Cc: anssi.kaariainen@… added
Has patch: set

In the attached patch the concrete_model._meta.local_fields are used to find out the fields to dump instead of obj._meta.local_fields.

by Anssi Kääriäinen, 12 years ago

Attachment: 17717.diff added

comment:3 by Claude Paroz, 12 years ago

Triage Stage: AcceptedReady for checkin

Looks good, thanks

comment:4 by Jannis Leidel, 12 years ago

Resolution: fixed
Status: newclosed

In [17640]:

Fixed #17717 -- Fixed serialization of proxy models. Thanks, Anssi Kääriäinen.

comment:5 by Simon Charette, 12 years ago

Now that there's a concrete_model property and that proxy_for_model is actually a proxy chain we should probably just get it from there?

Correct me if I'm wrong but the patch checked in r17640 will fail for proxy of a proxy model.

Should the patch be checked out and corrected with additional tests?

edit: I meant concrete_model and not concrete_class

Last edited 12 years ago by Simon Charette (previous) (diff)

comment:6 by Simon Charette, 12 years ago

Cc: charette.s@… added

comment:7 by Jannis Leidel, 12 years ago

Resolution: fixed
Status: closedreopened
Triage Stage: Ready for checkinAccepted

Oh, yeah, I didn't see the changes done in r17573, this new "concrete_model" property might indeed fix this easier. Re-opening..

by Simon Charette, 12 years ago

Make sure to rely on newly introduced concrete_model model option

comment:8 by Anssi Kääriäinen, 12 years ago

Triage Stage: AcceptedReady for checkin

Yes, that should be fixed. Now proxy_for_model is the actual proxied model, so as written the committed patch isn't correct any more. That is my failing, as I should have written it using the while opts._proxy_for_model: opts = opts._proxy_for_model._meta idiom instead of assuming the buggy behavior in the patch.

I tested the patch and without the fix the added test fails, with the fix in the patch it succeeds. I quickly skimmed through it and it seems ready for checkin to me.

Last edited 12 years ago by Anssi Kääriäinen (previous) (diff)

comment:9 by Jannis Leidel, 12 years ago

Resolution: fixed
Status: reopenedclosed

In [17643]:

Fixed #17717 (again) -- Used the new API for concrete models added in r17573. Many thanks to Simon Charette and Anssi Kääriäinen.

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