Opened 8 years ago

Closed 8 years ago

#4714 closed (fixed)

JSON serializer doesn't handle None primary keys correctly (has tests.)

Reported by: phil.h.smith@… Owned by: pigletto
Component: Core (Serialization) Version: master
Severity: Keywords: json serialization primary key None null
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Seems conceptually similar to #2894.

If you serialize('json', [model]) a model with model.id == None, you will produce json containing "pk": "None" instead of "pk": null. The offending line seems to be in django.core.serializers.python.Serializer.end_object, around line 29, when str(obj._get_pk_val()) is called (or this is intentional and the generated "None" needs to be handled further down the line).

Test cases attached.

Attachments (2)

json_serializer_should_deal_with_None_primary_keys.py (997 bytes) - added by phil.h.smith@… 8 years ago.
reproducing unit test
json.diff (7.8 KB) - added by pigletto 8 years ago.
Patch against revision 6193

Download all attachments as: .zip

Change History (6)

Changed 8 years ago by phil.h.smith@…

reproducing unit test

comment:1 Changed 8 years ago by Simon G. <dev@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Great - want to try to fix it?

comment:2 Changed 8 years ago by pigletto

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

Changed 8 years ago by pigletto

Patch against revision 6193

comment:3 Changed 8 years ago by pigletto

  • Has patch set
  • Triage Stage changed from Accepted to Ready for checkin

Patch contains tests but tests won't fail only if http://code.djangoproject.com/ticket/5448 is already checked in.
In other words, tests from this ticket are dependent of fix from 5448.

Original ticket was only about primary keys issue but I've found that there were some more unconsistences in serializer code.
In general 'pk' was always converted to unicode while primary keys from related fields were taken as is.

I've changed this behaviour that now every string is converted to unicode, while other types stay as they are.
One big deal with this path is that in some cases backward compatiblity may be broken!
It is because now, if you have number in primary key, it will be serialized as a number (in JSON: 'pk: 1). Previously it was always 'pk':"1".

comment:4 Changed 8 years ago by russellm

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

(In [6264]) Fixed #4714 -- Modified serializers to handle None primary keys correctly. This slightly changes the output format for primary keys on JSON and YAML serializers (PKs will be output as 1, rather than "1". However, fixtures in the old format will still load ok. Thanks for the patch, pigletto.

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