Opened 7 years ago

Closed 6 years ago

Last modified 4 years ago

#9522 closed (fixed)

Python serializer doesn't use base Serializer.get_string_value

Reported by: alexkoshelev Owned by: alexkoshelev
Component: Core (Serialization) Version: 1.0
Severity: Keywords:
Cc: Pike, chipx86 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 alexkoshelev 7 years ago.
new patch with tests

Download all attachments as: .zip

Change History (16)

comment:1 Changed 7 years ago by russellm

  • Needs documentation unset
  • 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 7 years ago by alexkoshelev

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

Changed 7 years ago by alexkoshelev

new patch with tests

comment:3 Changed 7 years ago by anonymous

  • Needs tests unset
  • Patch needs improvement unset

comment:4 Changed 7 years ago by Pike

  • Cc Pike added
  • Owner changed from nobody to Pike
  • Status changed from new to assigned

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 7 years ago by Pike

  • Owner changed from Pike to nobody
  • Status changed from assigned to new

comment:6 Changed 6 years ago by chipx86

  • Cc chipx86 added

comment:7 Changed 6 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:8 Changed 6 years ago by chipx86

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

  • milestone set to 1.1
  • Triage Stage changed from Unreviewed to Accepted

comment:10 Changed 6 years ago by ericholscher

  • 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 6 years ago by ericholscher

  • Owner changed from nobody to ericholscher
  • Status changed from new to assigned

comment:12 Changed 6 years ago by alexkoshelev

  • Owner changed from ericholscher to alexkoshelev
  • Patch needs improvement unset
  • Status changed from assigned to new

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

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

(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 6 years ago by russellm

(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 4 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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