Opened 4 years ago

Closed 5 months ago

#17587 closed Bug (duplicate)

serializing foreignkeys assumes value is serializable

Reported by: aburgel Owned by: nobody
Component: Core (Serialization) Version: 1.3
Severity: Normal Keywords:
Cc: aburgel Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

in django.core.serializers.python, handle_fk_field and handle_m2m_field assume that the foreignkey value requires no conversion, i.e. value_to_string is not called. handle_fk_field sometimes does not even convert the foreignkey value to a string (line 58).

if you're using custom fields whose values are not serializable, then you're given no opportunity for conversion, and in the case of json, if that value isn't a native json type, then you get an exception.

the way to work around this is to write your own serializer or to use natural keys, but given that value_to_string exists, it should be used.

Attachments (1)

fk_serialize.patch (5.4 KB) - added by aburgel 4 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 4 years ago by claudep

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

A test case would be nice, or at least an example model wich would surely fail to serialize properly.

comment:2 Changed 4 years ago by aburgel

  • Cc aburgel added

added a failing test case. i'm working on a fix as well.

comment:3 Changed 4 years ago by aburgel

added a patch with a test case and a fix for both python and xml serializers.

i didn't tackle the m2m case, but from the code it looks like you can only use a pk or a natural key, so maybe its not an issue.

comment:4 Changed 4 years ago by aburgel

  • Has patch set

btw, the patch is off the git mirror of the 1.3.X branch. let me know if i should create it off something different.

comment:5 Changed 4 years ago by claudep

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Yes, please provide the patch against master.

Changed 4 years ago by aburgel

comment:6 Changed 4 years ago by aburgel

ok, patched against master.

comment:7 Changed 5 months ago by claudep

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

Apparently, #24320 fixed this issue in 5c995dcfc251b55284e1ef16545acd2acad6be04.

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