#14823 closed (fixed)
Unexpected behavior with core.serializers.register_serializer and unregister_serializer
| Reported by: | Owned by: | nobody | |
|---|---|---|---|
| Component: | Core (Serialization) | Version: | dev |
| Severity: | Keywords: | ||
| Cc: | 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
Almost all the functions in serializers/__init__.py begin with the lines
if not _serializers:
_load_serializers()
but register_serializer and unregister_serializer do not.
When only using the built-in serializers you didn't have to manually register them but if you used an additional serializer you either have to
- Include it in settings.SERIALIZATION_MODULES (probably the "right way" to do things, but then why make register_serializer public?)
- call _load_serializers (non-public API)
- manually register both the built-ins and those listed in settings.SERIALIZATION_MODULES (duplicating _load_serializers)
- call any public function in
__init__.pyexcept register_serializer or unregister_serializer before registering (strange order of operations)
To reproduce (from a fresh python manage.py shell)
from django.core import serializers
serializers.register_serializer('json2', 'django.core.serializers.json')
serializers.get_public_serializer_formats() # ['json2']
serializers.get_serializer('xml') # KeyError: 'xml'
I've attached a patch that adds the same if not _serializers: check for register_serializer and unregister_serializer. It keeps with the general mentality of the existing code (e.g., not using a separate boolean value to check for registration) but introduces an edge case where code that attempts to unregister all built-in serializers before registering it's own will effectively do nothing. e.g.
from django.core import serializers
serializers.unregister_serializer('json') # loads json/python/xml (assume no yaml), then unregisters json
serializers.unregister_serializer('xml')
serializers.unregister_serializer('python') # _serializers is now an empty dict
serializers.register_serializer('foo', 'foo.bar.baz') # re-registers json/python/xml, then registers foo.
Under the existing code you'd get a KeyError when attempting to unregister json (since it hasn't been loaded yet).
Alternatively, it could be determined that register_serializer and unregister_serializer aren't stable API (they're not documented on the Serialization topic page). If that's the case it may make more sense to just prefix them with an _ and/or leave a note similar to the thread-safe notes.
Attachments (1)
Change History (5)
by , 15 years ago
| Attachment: | serializers__init__nowarning.diff added |
|---|
comment:1 by , 15 years ago
| Triage Stage: | Unreviewed → Ready for checkin |
|---|
comment:2 by , 15 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:3 by , 15 years ago
A slight correctly was necessary for this patch -- as provided, there is an infinite loop, because register_serializer called _load_serializers, which calls register_serializer, and so on.
I've also taken the liberty of adding some tests.
Simple patch for serializers/init.py