Opened 6 years ago

Closed 2 months ago

Last modified 2 months ago

#29522 closed Cleanup/optimization (fixed)

Make Serializers easier to modify.

Reported by: Levi Cameron Owned by: Amir Karimi
Component: Core (Serialization) Version: dev
Severity: Normal Keywords:
Cc: Herbert Fortes, Ian Foote, Ülgen Sarıkavak 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 (last modified by Levi Cameron)

Use case

Three times now I've had need to modify the ORM serializers for project use. In some cases I've had to replicate large parts (if not most) of django.core.serializers.python in order to change 1 or 2 lines because certain calls to functions in django.core.serializers.base from the serializers are hardcoded.

  • Changing JSON serialization to output in alphabetical order (so that dumpdata doesn't reorder every field each time you run it)
  • Changing deserialization to use _base_manager instead of _default_manager (due to extra constraints on a custom manager)
  • Implementing fixes for #7202, #7350 before they were merged

Proposal
I propose that instead of calling the following functions directly, the serializer classes are modified to call an instance method of the same name that then calls the existing module-level functions. This would allow the ability to more easily override just one aspect of serializer functionality without having to replicate large swathes of code.

I am not proposing to move these functions into the python serializer class directly because this may break compatibility with existing code that depends on the existing functions.

  • django.core.serializers.base.build_instance
  • django.core.serializers.base.deserializer_m2m_values
  • django.core.serializers.base.deserializer_fk_values
  • django.core.serializers.python._get_model (given that this is already an internal function, implementation can be pushed into the class directly)

(It might also be nice if the json and pyyaml classes could also have PythonDeserializer be made a class attribute but that would involve turning the Deserializer function into a class and is not nearly as much code to override these as the functions above)

If this sounds acceptable, I will submit a patch.

Change History (26)

comment:1 by Levi Cameron, 6 years ago

Description: modified (diff)

comment:2 by Levi Cameron, 6 years ago

Description: modified (diff)

comment:3 by Levi Cameron, 6 years ago

Description: modified (diff)

comment:4 by Carlton Gibson, 6 years ago

Triage Stage: UnreviewedAccepted

This sounds reasonable to me. My main thought would be to make sure the new hooks a properly documented etc. But yes, good idea.

comment:5 by Carlton Gibson, 6 years ago

Summary: Serializers are hard to modifyMake Serializers easier to modify

comment:6 by Herbert Fortes, 6 years ago

Cc: Herbert Fortes added

comment:7 by Emad Mokhtar, 6 years ago

Owner: changed from nobody to Emad Mokhtar
Status: newassigned

comment:8 by Ian Foote, 6 years ago

Cc: Ian Foote added

comment:9 by Emad Mokhtar, 6 years ago

Version 1, edited 6 years ago by Emad Mokhtar (previous) (next) (diff)

comment:10 by Ian Foote, 6 years ago

Has patch: set

comment:11 by Mariusz Felisiak, 6 years ago

Needs tests: set
Patch needs improvement: set
Summary: Make Serializers easier to modifyMake Serializers easier to modify.

comment:12 by Mariusz Felisiak, 8 months ago

Owner: Emad Mokhtar removed
Status: assignednew

comment:13 by Ülgen Sarıkavak, 8 months ago

Cc: Ülgen Sarıkavak added

comment:14 by Amir Karimi, 5 months ago

Needs tests: unset
Owner: set to Amir Karimi
Patch needs improvement: unset
Status: newassigned

comment:15 by Sarah Boyce, 3 months ago

Patch needs improvement: set

comment:16 by Amir Karimi, 2 months ago

Patch needs improvement: unset

comment:17 by Sarah Boyce, 2 months ago

Patch needs improvement: set

comment:18 by Amir Karimi, 2 months ago

Patch needs improvement: unset

comment:19 by Sarah Boyce, 2 months ago

Patch needs improvement: set

comment:20 by Amir Karimi, 2 months ago

Patch needs improvement: unset

comment:21 by Sarah Boyce <42296566+sarahboyce@…>, 2 months ago

In b250175:

Refs #29522 -- Improved test coverage of deserializers.

comment:22 by Sarah Boyce, 2 months ago

Patch needs improvement: set

comment:23 by Amir Karimi, 2 months ago

Patch needs improvement: unset

comment:24 by Sarah Boyce, 2 months ago

Triage Stage: AcceptedReady for checkin

comment:25 by Sarah Boyce <42296566+sarahboyce@…>, 2 months ago

Resolution: fixed
Status: assignedclosed

In ee5147cf:

Fixed #29522 -- Refactored the Deserializer functions to classes.

Co-authored-by: Emad Mokhtar <emad.mokhtar@…>

comment:26 by Sarah Boyce <42296566+sarahboyce@…>, 2 months ago

In 1fa84936:

Refs #29522 -- Fixed serializers/fixtures test crash if PyYAML isn't installed.

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