Opened 8 years ago

Closed 8 years ago

Last modified 5 years ago

#9522 closed (fixed)

Python serializer doesn't use base Serializer.get_string_value

Reported by: Alexander Koshelev Owned by: Alexander Koshelev
Component: Core (Serialization) Version: 1.0
Severity: Keywords:
Cc: Pike, Christian Hammond Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Just now python serializer in handle_field simple call getattr to retrieve field value form the object:

smart_unicode(getattr(obj, field.name), strings_only=True)

And base Serializer has method get_string_value that calls appropriate value_to_string field method but derived serializers doesn't call it at all.

It causes problem than model has non-string field that cannot simply be converted to string.

Attachments (1)

9522.patch (3.8 KB) - added by Alexander Koshelev 8 years ago.
new patch with tests

Download all attachments as: .zip

Change History (16)

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

Needs tests: set
Patch needs improvement: set

Quick and dirty is fine, but you're not going to convince me to apply the patch without a test case - ideally, something integrated into the Django test suite, but at the very least, a model definition and python session that demonstrates the problem.

comment:2 Changed 8 years ago by Alexander Koshelev

Yes, first attempt was not good. I've made some more changes and written the tests.

Changed 8 years ago by Alexander Koshelev

Attachment: 9522.patch added

new patch with tests

comment:3 Changed 8 years ago by anonymous

Needs tests: unset
Patch needs improvement: unset

comment:4 Changed 8 years ago by Pike

Cc: Pike added
Owner: changed from nobody to Pike
Status: newassigned

I'm getting hit by this, too.

I found that the DeSerialization calls into to_python [1], so I wonder if the Serialization shouldn't just call into get_db_prep_save.

Or is it a safe assumption that feeding a string into to_python is always going to succeed?

[1] http://code.djangoproject.com/browser/django/trunk/django/core/serializers/python.py#L88

comment:5 Changed 8 years ago by Pike

Owner: changed from Pike to nobody
Status: assignednew

comment:6 Changed 8 years ago by Christian Hammond

Cc: Christian Hammond added

comment:7 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:8 Changed 8 years ago by Christian Hammond

Are there any plans to get this in? It's proving to be a real problem for me. After some Googling, it seems several people are patching their Django sources to work around this.

comment:9 Changed 8 years ago by Jacob

milestone: 1.1
Triage Stage: UnreviewedAccepted

comment:10 Changed 8 years ago by Eric Holscher

Patch needs improvement: set

This patch is adding a value to every model in Django. I don't think this is the correct implementation. Models know nothing about serialization, and if we're going to start adding serialization information to them, we're going to need to have a much bigger discussion about it.

This change should be contained to the serializers, perhaps with a third party serializer that provides the functionality. Something along the lines of ./manage.py dumpdata --full-serialize=ModelName perhaps.

comment:11 Changed 8 years ago by Eric Holscher

Owner: changed from nobody to Eric Holscher
Status: newassigned

comment:12 Changed 8 years ago by Alexander Koshelev

Owner: changed from Eric Holscher to Alexander Koshelev
Patch needs improvement: unset
Status: assignednew

Oh, where do you find "adding value to every model"? Patch doesn't do it at all. It only adds flag attribute to model *field* and field already knows how to convert it to string. And this knowledge is used in serializers just now.

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

Resolution: fixed
Status: newclosed

(In [10554]) Fixed #9522 -- Modified handling of values in base serializer so that field subclasses can define their own value_to_string() method for serialization. Thanks to Alex Koshelev for the report and initial patch.

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

(In [10555]) [1.0.X] Fixed #9522 -- Modified handling of values in base serializer so that field subclasses can define their own value_to_string() method for serialization. Thanks to Alex Koshelev for the report and initial patch.

Merge of r10554 from trunk.

comment:15 Changed 5 years ago by Jacob

milestone: 1.1

Milestone 1.1 deleted

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