#32420: build_instance in (de)serializers uses model's primary key field name
instead of attname to detect if data contains a primary key
------------------------------------------------+--------------------------
Reporter: trybik | Owner: trybik
Type: Bug | Status: assigned
Component: Core (Serialization) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+--------------------------
The bug was introduced in #28385 fix, in
[https://github.com/django/django/commit/dcd1025f4c03faa4a9915a1d47d07008280dd3cf
commit dcd1025]. Currently in master, the
{{{django.core.serializers.base.build_instance}}} function uses {{{pk =
data.get(Model._meta.pk.name)}}} instead of {{{pk =
data.get(Model._meta.pk.attname)}}}:
{{{
def build_instance(Model, data, db):
"""
Build a model instance.
If the model instance doesn't have a primary key and the model
supports
natural keys, try to retrieve it from the database.
"""
default_manager = Model._meta.default_manager
pk = data.get(Model._meta.pk.name)
if (pk is None and hasattr(default_manager, 'get_by_natural_key') and
hasattr(Model, 'natural_key')):
natural_key = Model(**data).natural_key()
try:
data[Model._meta.pk.attname] = Model._meta.pk.to_python(
default_manager.db_manager(db).get_by_natural_key(*natural_key).pk
)
except Model.DoesNotExist:
pass
return Model(**data)
}}}
([https://github.com/django/django/blob/738e9e615dc81b561c9fb01439119f4475c2e25b/django/core/serializers/base.py#L260
Source on GitHub])
Why it should use {{{attname}}}? Because it may be different than
{{{name}}} and the following corresponding sets use
{{{data[Model._meta.pk.attname] = ...}}}:
1. proceeding {{{django.core.serializers.python.Deserializer}}} which
calls {{{build_instance}}}:
{{{
def Deserializer(...):
...
for d in object_list:
...
data = {}
if 'pk' in d:
try:
data[Model._meta.pk.attname] =
Model._meta.pk.to_python(d.get('pk'))
except Exception as e:
...
obj = base.build_instance(Model, data, using)
yield ...
}}}
([https://github.com/django/django/blob/738e9e615dc81b561c9fb01439119f4475c2e25b/django/core/serializers/python.py#L100
Source on GitHub])
2. following 5 lines further assignment {{{ data[Model._meta.pk.attname] =
...}}} uses {{{attname}}}.
The marginal error case is when primary key is also a foreign key (then
{{{attname}}} has {{{"_id"}}} suffix). Moreover, to actually make it an
error you have to create a bit pathological situation where you have to
have natural key but it does not work, e.g.:
{{{
class Author(models.Model):
name = models.CharField(max_length=20)
class GhostWriterManager(models.Manager):
def get_by_natural_key(self, *args, **kwargs):
raise NotImplementedError("Don't get by natural key")
class GhostWriter(models.Model):
author_ptr = models.ForeignKey(Author, on_delete=models.CASCADE,
primary_key=True)
objects = GhostWriterManager()
def natural_key(self):
raise NotImplementedError("There is no natural key")
}}}
This rare situation actually can come up in an arguably normal situation,
when using django-polymorphic and loading the subclass part of JSON list
and the subclass uses a natural key that refers to fields from base class.
The natural key will work perfectly fine, just not when loading the
subclass part of JSON.
--
Ticket URL: <https://code.djangoproject.com/ticket/32420>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/049.d33428845cc5df42ce7661c32a37bc0a%40djangoproject.com.
